On Fri, 19 Dec 2025 15:38:29 GMT, Christian Hagedorn <[email protected]> wrote:
> 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 V... Looks reasonable to me. Disabling all the Valhalla flags when preview mode is not enabled would reduce the risks for the normal mode to encounter a bug introduced by Valhalla, which would be very important after the integration with mainline. ------------- Marked as reviewed by fparain (Committer). PR Review: https://git.openjdk.org/valhalla/pull/1846#pullrequestreview-3599272660
