RFR: L/Q again, step two [v2]

[Tobias Hartmann]

On Tue, 18 May 2021 21:02:40 GMT, Frederic Parain <fparain at openjdk.org> wrote:

>> Please review this second set of fixes for the L/Q transition.
>> A few changes in C1, most changes in CI.
>> CI always delegates to the runtime to know if a null-free array is flattened or not (to avoid duplication of the flattening decision code).
>> 
>> With those changes, all tests in runtime/valhalla/inlinetypes now pass with -Xcomp -XX:TieredStopAtLevel=1 (at least on Linux x64).
>> Some tests in compiler/valhalla/inlinetypes still fail but they will require more time to be investigated (some tests force use of C2 or expect code to be C2 compiled, but C2 is not fixed yet).
>> 
>> Thank you,
>> 
>> Fred
>
> Frederic Parain has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Change error message in test

Looks good to me. Added some minor comments.

src/hotspot/share/ci/ciArrayKlass.cpp line 124:

> 122:         } else {
> 123:           if (ak != NULL && ak->is_flatArray_klass()) {
> 124:             is_array_flattened = true;

Why is that boolean needed? Can't we simply return `ciFlatArrayKlass::make(klass)` here?

src/hotspot/share/ci/ciArrayKlass.cpp line 134:

> 132:       }
> 133:     } else {
> 134:       return ciEnv::unloaded_ciobjarrayklass();

Can't we delegate handling the unloaded case to `ciObjArrayKlass::make(klass)` below? I.e. check for `if (null_free && klass->is_loaded())` above.

src/hotspot/share/ci/ciEnv.cpp line 478:

> 476:     if (elem_klass != NULL && elem_klass->is_loaded()) {
> 477:       // Now make an array for it
> 478:       bool null_free_array = sym->is_Q_array_signature() && sym->char_at(1) == JVM_SIGNATURE_INLINE_TYPE;

Why do we need the `sym->char_at(1) == JVM_SIGNATURE_INLINE_TYPE` check? Isn't that part of `is_Q_array_signature()`?

-------------

Changes requested by thartmann (Committer).

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