[9] RFR (M): 8143407: C1: @Stable array support

Vladimir Ivanov vladimir.x.ivanov at oracle.com
Fri Feb 26 21:05:27 UTC 2016


Thanks for the feedback, Chris.

Updated webrev in place:
   http://cr.openjdk.java.net/~vlivanov/8143407/webrev.01
On 2/26/16 10:27 PM, Christian Thalinger wrote:
>            // Stable static fields are checked for non-default values in ciField::initialize_from().
> +         assert(!field->is_stable() || !field_value.is_null_or_zero(), "should not be a constant");
>
> I don’t understand this assert.  Especially the message text.
It checks an invariant between ciField::is_constant/constant_value & 
ciField::is_stable(). For static fields constant folding happens when 
ciField instance is constructed, so the assert ensures that for a stable 
static field its default value is never seen as a constant.

I slightly reworded the assert message. Hope it reads better now:
+         assert(!field->is_stable() || !field_value.is_null_or_zero(),
+                "stable static w/ default value should not be a constant");
+

> +   switch (field_type) {
> +     case T_ARRAY:
> +     case T_OBJECT:
> +       if (field_value.as_object()->should_be_constant()) {
> +         return new Constant(value);
> +       }
> +       break;
> +     default:
> +      return new Constant(value);
> +   }
> +   return NULL; // Not a constant.
>
> Could you move the return NULL up to where the break is?  Would make it easier to read.
Ok.

> +            if (FoldStableValues && field->is_stable() && field_value.is_null_or_zero()) {
>                 // Stable field with default value can't be constant.
> -              constant = NULL;
>
> I understand that the null assignment is not needed but it would be nice to keep it.
Ok.

Best regards,
Vladimir Ivanov


More information about the hotspot-compiler-dev mailing list