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... Thanks Fred for your review! > 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. Yes, absolutely, I agree with that. ------------- PR Comment: https://git.openjdk.org/valhalla/pull/1846#issuecomment-3675756903
