On Mon, 3 Nov 2025 14:12:55 GMT, Manuel Hässig <[email protected]> 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

Reply via email to