### Failing Test The assert added in mainline to verify that `adr_type` and `adr`'s type match fails in Valhalla when facing a load from a cloned flat array (i.e. called `clone()`) in `TestArrays::test28()`: https://github.com/openjdk/valhalla/blob/cfda30f3735276318c291718ca1f3daaa0e124c8/test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestArrays.java#L808-L813
This triggers when trying to add array elements to safepoints. ### Mismatched `AddP` Type vs. Calculated `adr_type` In `PhaseMacroExpand::make_arraycopy_load()`, we have the following type for the `base` of the flat array `src`: // gvn.type(base) with _offset = 0, _field_offset = bottom: aryptr:flat:instptr:.../MyValue2 (...):NotNull:exact *,iid=bot[int:10] (...):NotNull:exact:flat(+bot):null free We then try to add the flat array load for `src.x` which is at `offset = 32` (16 bytes for the array object header + another 16 bytes to get to `x` at offset 16 (`src.v` is first at offset 0)). Here we get a mismatch between the computed `adr_type` and the type stored for `adr` (i.e. an `AddP`) which is later used when creating the `LoadNode`: // adr_type with _offset = 32, _field_offset = bottom: aryptr:flat:instptr:.../MyValue2 (...):NotNull:exact *,iid=bot[int:10] (...):NotNull:exact:flat(+bot):null free[2] // gvn.type(adr) with _offset = 16, _field_offset = 16: aryptr:flat:instptr:.../MyValue2 (...):NotNull:exact *,iid=bot[int:10] (...):NotNull:exact:flat(+16):null free[0] The types have different `_field_offset` values and thus end up on a different alias class which triggers the assert. ### Reason for Mismatch The problem is that in `PhaseMacroExpand::make_arraycopy_load()`, we simply call `add_offset()`, regardless of whether it's a flat array or not: https://github.com/openjdk/valhalla/blob/cfda30f3735276318c291718ca1f3daaa0e124c8/src/hotspot/share/opto/macro.cpp#L323-L326 This ignores `_field_offset` and leaves it at `bottom`. But when calling `AddPNode::Value()`, we call `TypeAryPtr::add_field_offset_and_offset()` which also considers `_field_offset` and properly sets it to 16 and correspondingly `_offset` to 16. ### Fixing the Mismatch A straight forward fix is to use `add_field_offset_and_offset()` instead of `add_offset()` when having an array pointer. This works to address the observing assertion failure. This is what's suggested in this patch. I also applied some small clean-ups. ### Another Lurking Bug In `add_field_offset_and_offset()`, we have special code to find out what the `_field_offset` is for a flat array pointer: https://github.com/openjdk/valhalla/blob/583bdbc45a0c044eb0e26436983e9f416f806ed6/src/hotspot/share/opto/type.cpp#L5799-L5809 This assumes that `_field_offset` is implicitly 0 even when it's set to `bottom` which could actually mean any field offset. This most likely seems to hold in all the cases we currently know of but is not correct in the general case. We should fix that as well. But when restricting this to `_field_offset != bottom`, we hit other assertions. This requires more investigation. To move forward with this patch and enable more testing again, I suggest to fix this second issue separately, either with [JDK-8358079](https://bugs.openjdk.org/browse/JDK-8358079) or a new issue. Thanks, Christian --------- - [x] I confirm that I make this contribution in accordance with the [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). ------------- Commit messages: - Fix Changes: https://git.openjdk.org/valhalla/pull/2370/files Webrev: https://webrevs.openjdk.org/?repo=valhalla&pr=2370&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8381268 Stats: 35 lines in 3 files changed: 10 ins; 5 del; 20 mod Patch: https://git.openjdk.org/valhalla/pull/2370.diff Fetch: git fetch https://git.openjdk.org/valhalla.git pull/2370/head:pull/2370 PR: https://git.openjdk.org/valhalla/pull/2370
