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