[lworld] RFR: 8245584: [lworld] LW3 core reflection update

Frederic Parain fparain at openjdk.java.net
Fri May 22 19:56:04 UTC 2020

On Thu, 21 May 2020 22:09:50 GMT, Mandy Chung <mchung at openjdk.org> wrote:

> This patch updates the core reflection for the new language model for inline classes.
> - `Class::isInlineClass` returns true for inline classes
> - `Class::valueType` returns the value projection of an inline class
> - `Class::referenceType` returns the reference projection if it's an inline class,
>     or this class if it's not an inline class
> If this class has no val type or ref type, `Class::valueType` and `Class::referenceType`
> return an empty `Optional`.   We can re-examine  if we should modernize Class API shape
> where  we can rather than holding to the tradition.
> I updated TestIntrinsics.java to use the new APIs for now to keep it compiled.
> This test will be updated when the intrinsification of the old API `Class::asPrimaryType`
> and `Class:asIndirectType` is removed.

Hi Mandy,

The HotSpot changes look good in general, with a lot of nice cleanups.
You'll find two comments, one minor about hard-coded characters, and a second more significant about the
`Class::refType` field, for which you might want a review from the compiler team.

I find it disturbing to have VM code depending on value projection/reference projection fields when these are language
concepts that are not part of the VM model. but this has nothing to do with your changes, it might be due to the model
transition, or something we missed in the VM model.

Thank you,


src/hotspot/share/oops/instanceKlass.cpp line 2775:

> 2774:   int dest_index = 0;
> 2775:   dest[dest_index++] = is_value() ? 'Q' : 'L';
> 2776:

Should use `JVM_SIGNATURE_CLASS` and `JVM_SIGNATURE_VALUETYPE` instead of hard-coded characters.

src/hotspot/share/classfile/javaClasses.cpp line 1023:

> 1022:         set_ref_type_mirror(ref_type_oop, ref_type_oop);
> 1023:       }
> 1024:     }

With this code, if the super type of the inline type is `j.l.Object`, `Class:refType` will remain `null`. I'm not sure
that the VM code is ready to handle this situation. The only use case I've found is in
`LibraryCallKit::inline_value_Class_conversion(vmIntrinsics::ID id)` and I'm not familiar with it (the compiler team
might want to take a look at it).


Marked as reviewed by fparain (Committer).

PR: https://git.openjdk.java.net/valhalla/pull/53

More information about the valhalla-dev mailing list