[lworld] RFR: 8367244: [lworld] C2 compilation fails with assert "no node with a side effect"

[Christian Hagedorn]

On Mon, 3 Nov 2025 14:12:55 GMT, Manuel Hässig <mhaessig at openjdk.org> wrote:

> ## Problem Analysis
> 
> The stress test `compiler/valhalla/inlinetypes/TestAcmpWithUnstableIf.java`, which uses the `StressUnstableIfTrap` flag, failed intermittently with the assert "no node with a side effect" during C2 compilation. I tracked down the origin of the failure to the raw store of the unstable if trap stress counter that was missing a memory edge to the backedge phi and thus had no side effect in the loop, which lead to the aforementioned assert. During parsing, the missing memory edge gets discarded with as vestigial parsing state when `do_if()` ends up `stopped()`. However, the effect of the stress counter should still be wired back into the backedge since it will be incremented in the next iteration.
> 
> ## Patch Description
> 
> To prevent the stress counters memory state being lost to `PreserveJVMState`, I pass it to the caller over an out-parameter and `set_memory` with that node, similar to what is already done for control. This is only necessary for the two last invocation of `do_if()` in `do_acmp()` where they are actually allowed to trap. Further, this PR reenables `StressUnstableIfTrap` in `compiler/valhalla/inlinetypes/TestAcmpWithUnstableIf.java`.
> 
> ## Testing
> 
> - [x] tier1,tier2,tier3 plus internal stress testing

Looks reasonable to me, thanks for fixing this!

test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestAcmpStressUnstableIf.java line 36:

> 34:  * @run main/othervm -Xbatch -XX:-TieredCompilation
> 35:  *                   -XX:CompileCommand=compileonly,compiler/valhalla/inlinetypes/TestAcmpStressUnstableIf.test
> 36:  *                   -XX:+UnlockDiagnosticVMOptions -XX:+StressUnstableIfTraps -XX:RepeatCompilation=100

Given that the test will be run over and over again, I think we can remove `RepeatCompilation` to improve test execution time.

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

Marked as reviewed by chagedorn (Committer).

PR Review: https://git.openjdk.org/valhalla/pull/1716#pullrequestreview-3422589358
PR Review Comment: https://git.openjdk.org/valhalla/pull/1716#discussion_r2494886907