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

[Christian Thalinger]

> On Feb 26, 2016, at 11:05 AM, Vladimir Ivanov <vladimir.x.ivanov at oracle.com> wrote:
> 
> 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”);

Better, thanks.  Are we saving bytes in assert messages now? ;-)

> +
> 
>> +   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);

Just noticed the indent is wrong on this line.

>> +   }
>> +   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