RFR (S) 8150102: C1 should fold arraylength for constant/trusted arrays

[Vladimir Ivanov]

>> Why don't you fold as_LoadField into the assert?
>
> Because "lf->field()" is polled from as_LoadField:
>
>    } else if ((lf = x->array()->as_LoadField()) != NULL) {
>      // GraphBuilder should emit Constants in static final case,
>      //let's assert that.
>      ciField* field = lf->field();
>      assert (!(field->is_constant() && field->is_static()),
>              "Array loads from static final fields should be handled as
> Constants");
>    }
>
> Granted, we can construct a convoluted short-circuiting boolean
> expression that folds that, but I think explicitly matching the "type"
> is cleaner (sucks that our code does not have proper pattern matching).
Agree, it doesn't look pretty when both is_constant() && is_static() 
checks are present.

Maybe structure it as follows?
   } else {
#ifdef ASSERT
     LoadField* lf = x->array()->as_LoadField();
     bool is_static_constant = (lf != NULL) && 
lf->field()->is_constant() && lf->field()->is_static();
     assert(!is_static_constant, "constant field loads are folded during 
parsing");
#endif // ASSERT
   }

But choose whatever version you like. I don't have a strong opinion here.

>> Also, field->is_static() is redundant: ciField::initialize_from does all
>> necessary checks.
>
> No, it isn't redundant: is_constant() is TRUE for trusted non-static
> finals, like String.value. The old code predicated on is_static() to
> pull the constant value without having an instance to introspect.
> Perhaps, we need to fold (is_constant == TRUE, is_static == FALSE) loads
> during parsing too to make a stronger assertion?
Yes, you are right.

>   http://cr.openjdk.java.net/~shade/8150102/webrev.02/
Looks good.

Best regards,
Vladimir Ivanov