RFR 8220269 [lworld] C1 should return default value for uninitialized non-static Q fields

Ioi Lam ioi.lam at oracle.com
Fri Mar 8 03:57:24 UTC 2019



On 3/6/19 9:32 PM, Ioi Lam wrote:
> https://bugs.openjdk.java.net/browse/JDK-8220269
> http://cr.openjdk.java.net/~iklam/valhalla/8220269-getfield-uninited-jumbo-qfield.v01/ 
>
>
> For the following bytecode
>
>     getfield Holder.f:"QJumboValue"
>
> If the field is too big to be flattened (e.g., ciField::is_flattened()
> returns false), we should load the field as an oop, test for null, and
> replace with JumboValue.default.
>
> ---
>
> I also refactored the handling of unloaded classes by getfield/getstatic
> into a new function LIRGenerator::non_nullable_load_field_prolog(). I 
> needed to
> add a new method:
>
>   bool ciField::is_never_null() const {
>     // Cannot use (type()->basic_type() == T_VALUETYPE) -- if the 
> class is
>     // not loaded, type() is an unloaded ciInstanceKlass!
>     return signature()->char_at(0) == 'Q';
>   }
>

Per John Rose's comments (offline), I renamed this function to 
ciField::is_q_type(), as there's a chance in the future that "Q" doesn't 
necessary mean "not nullable".


> I think putfield also need to be fixed, because it currently rely on
> ciField::type(), which essentially changes "Q" to "L" for unloaded 
> value classes.
>
> But I will do that in a separate JBS issue.
>

I double checked the code and the tests, and we already correctly handle 
the case where putfield is used with an unloaded valuetype. The test 
cases are TestUnloadedValueTypeField::test{4,5}. I corrected the 
comments and and added an assert in LIRGenerator::do_StoreField().

Delta from the last webrev

http://cr.openjdk.java.net/~iklam/valhalla/8220269-getfield-uninited-jumbo-qfield.v02-delta/

Thanks
- Ioi



More information about the valhalla-dev mailing list