There are currently a lot of failures in the CI with the following assert:

 assert(UseArrayFlattening && is_reference_type(bt) && 
element_ptr->can_be_inline_type() && (!element_ptr->is_inlinetypeptr() || 
element_ptr->inline_klass()->maybe_flat_in_array())) failed: array can't be flat


The reason is that C2 is using `UseArrayFlattening` to decide if some array 
type is known to be not flat. For example here:
https://github.com/openjdk/valhalla/blob/69399cedf6fe208832a66c134d370af860154bc2/src/hotspot/share/opto/library_call.cpp#L5197-L5198


When `--enable-preview` is not provided, we know that nothing is flat (i.e. all 
array types are known to be not flat). However, since `UseArrayFlattening` is 
still set to true by default, the C2 code above, for example, checks the flag 
and then assumes some array is not known to be non-flat. This clashes with 
other code that does an `--enable-preview` check as for example here:
https://github.com/openjdk/valhalla/blob/69399cedf6fe208832a66c134d370af860154bc2/src/hotspot/share/ci/ciInstanceKlass.cpp#L729-L732

and we get inconsistent results, leading to the assertion failure below.

We could fix all the places where we use Valhalla specific flags and make sure 
it works without setting `--enable-preview`. But I propose the easier fix to 
just disable Valhalla specific flags when `--enable-preview` is not set. This 
also protects us from future uses of the flag in shared code.

#### Additional Fix
`LibraryCallKit::inline_array_copyOf()` unconditionally sets "not-null-free" of 
a `TypeAryKlassPtr` to false because we don't know anything about it. However, 
when we run without `--enable-preview`, then we will not have null-free arrays. 
Therefore, we can set it to true in this case. When we don't do that, we will 
later assert where we check that when an array is NOT not-null-free, then we 
must be dealing with some kind of value class, which is not the case.

#### Implementation
I defined local macros in order to set the Valhalla flags to false and emit a 
warning when we set any of the flags added for Valhalla that they don't have an 
effect.

#### Future Work
We might want to add more sanity assertion checks for flat and nullness 
information when we create new type instances in the code. But since this is a 
bug fix for the CI, I'm leaving that as follow-up work.

#### Testing
- t1-3 (still running)
- previously failing t4 AOT tests (still running)

#### Alternative Fix
Fix all the mismatched cases directly in the code and allow Valhalla flags to 
be set even though they have no meaning without `--enable-preview`. We could 
also do that but it felt safer to just disable all the Valhalla specific flags 
without `--enable-preview`.

Thanks,
Christian

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

Commit messages:
 - new line
 - update warning message
 - not-null-free fix
 - 8374117: [lworld] Disable Valhalla specific VM flags when --enable-preview 
is not set

Changes: https://git.openjdk.org/valhalla/pull/1846/files
  Webrev: https://webrevs.openjdk.org/?repo=valhalla&pr=1846&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8374117
  Stats: 50 lines in 2 files changed: 38 ins; 0 del; 12 mod
  Patch: https://git.openjdk.org/valhalla/pull/1846.diff
  Fetch: git fetch https://git.openjdk.org/valhalla.git pull/1846/head:pull/1846

PR: https://git.openjdk.org/valhalla/pull/1846

Reply via email to