That was discovered when working on the intrinsic for isAtomic. It turns out it is actually not needed, but the shape of the graph to trigger the issue needs to be complex enough not to be simplified too hard, but simple enough that optimization triggers.
Let's see what happens. In this screenshot, let's look at <img width="2520" height="1935" alt="1 before" src="https://github.com/user-attachments/assets/e6e1d0d1-2034-49b6-948c-138c23f56d48"; /> Nodes `569 LoadB` and `573 LoadB` are loading the fields from `nullFreeAtomicArray2[1]`, in the inlined `Asserts.assertEquals`. Since they are strict final fields, they cannot be observed to be mutated, and we can rewire the memory input a lot higher. We get <img width="1601" height="1841" alt="2 after" src="https://github.com/user-attachments/assets/cec607fd-16e8-4462-881d-658a5200bd45"; /> The difficulty comes from the node `563 LoadN` that corresponds to the access `nullFreeAtomicArray2[1]`: we can climb up to the initial value of the `355 AllocateArray`. <img width="1750" height="910" alt="3 getting input" src="https://github.com/user-attachments/assets/0b0bb92a-4d7d-40c9-a24d-847245f8f52a"; /> Here, the default value is the value `initVal1`, which is `340 LoadN`, and the type is opaque here. Then, the graph becomes <img width="1719" height="777" alt="4 nope" src="https://github.com/user-attachments/assets/ad1aa005-a77a-4c26-a65c-651c48d82cf5"; /> then <img width="539" height="427" alt="5 nope" src="https://github.com/user-attachments/assets/b41c119e-e0a5-4fcb-8ee5-1a30ae718389"; /> and here, it is not clear that these `LoadB` are for strict final fields. That starts to be wrong. Later, during GCM, precedence edges from `538 MemBarStoreStore` to the `LoadB` nodes, creating a cycle inside the same block, and then, the graph becomes not schedulable. The fix is either not to subsume `563 LoadN` by `340 LoadN` (which is sad) because types don't match, or subsumes it by something correct. Here, we can just add a `CheckCastPPNode` under the initial value to subsumes by something correct, and all is fine again. Should we check that the initial value indeed has the right type? Yes, but not here, and it is worked on (see [JDK-8383820: [lworld] Several native methods of ValueClass class lack proper argument validation](https://bugs.openjdk.org/browse/JDK-8383820)). I've just added a debug only check and halt, it shall be enough for now. Thanks, Marc --------- - [x] I confirm that I make this contribution in accordance with the [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). ------------- Commit messages: - Fix test copyright - Explain the case a bit more - Fix test - Guard for init_val == nullptr - Add a checkcast Changes: https://git.openjdk.org/valhalla/pull/2432/files Webrev: https://webrevs.openjdk.org/?repo=valhalla&pr=2432&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8384127 Stats: 90 lines in 2 files changed: 86 ins; 0 del; 4 mod Patch: https://git.openjdk.org/valhalla/pull/2432.diff Fetch: git fetch https://git.openjdk.org/valhalla.git pull/2432/head:pull/2432 PR: https://git.openjdk.org/valhalla/pull/2432
