RFR 8219242 [lworld] C1 aastore on a Phi node needs flattened array check

Ioi Lam ioi.lam at oracle.com
Thu Feb 21 04:17:39 UTC 2019


Hi Tobias, here's an updated version with your suggestions:

http://cr.openjdk.java.net/~iklam/valhalla/8219242-aastore-phi-node-needs-flat-check.v02/

Note that I changed this line

1687   bool obj_store = x->elt_type() == T_OBJECT; assert(x->elt_type() 
!= T_ARRAY, "never used");

because x->elt_type() is determine by the bytecode (Xastore) and no 
bytecode will set it to T_ARRAY.

Thanks
- Ioi

On 2/18/19 11:43 PM, Tobias Hartmann wrote:
> Hi Ioi,
>
> I assume the value->type()->is_object() check is to avoid emitting checks when we are storing a
> primitive? But what if 'value' has an exact type? For example, it could be java.lang.Integer and in
> this case we don't need a flattened array check.
>
> Also, it's confusing that you pass 'x' as value in line 2061.
>
> What if ValueArrayFlatten is disabled? needs_flattened_array_check() would still return true. I
> would suggest to move the array->as_Phi check into 'maybe_flattened_array'.
>
> Best regards,
> Tobias
>
> On 18.02.19 16:31, Ioi Lam wrote:
>> http://cr.openjdk.java.net/~iklam/valhalla/8219242-aastore-phi-node-needs-flat-check.v01/
>> https://bugs.openjdk.java.net/browse/JDK-8219242
>>
>> With this fix, all current C1 compiler test cases have passed :-)
>>
>> My plan is to now enable the other tests that have been disabled for C1, and keep going ....
>>
>> Thanks
>> - Ioi
>>



More information about the valhalla-dev mailing list