[lworld] RFR: 8361465: [lworld] testlibrary_tests/ir_framework/tests/TestPhaseIRMatching.java fails

[Marc Chevalier]

On Mon, 14 Jul 2025 11:55:06 GMT, Marc Chevalier <mchevalier at openjdk.org> wrote:

> The constraint `failOn = IRNode.STORE` on `NoCompilationOutput.badPhase2` in `test/hotspot/jtreg/testlibrary_tests/ir_framework/tests/TestPhaseIRMatching.java` does not fail for the phase `ITER_GVN_AFTER_ELIMINATION`:
> 
> AFTER: ITER_GVN_AFTER_ELIMINATION
>   0  Root  === 0 21  [[ 0 1 3 ]]
>   3  Start  === 3 0  [[ 3 5 6 7 8 9 ]]  #{0:control, 1:abIO, 2:memory, 3:rawptr:BotPTR, 4:return_address, 5:ir_framework/tests/NoCompilationOutput:NotNull *}
>   5  Parm  === 3  [[ 21 ]] Control !jvms: NoCompilationOutput::badPhase2 @ bci:-1 (line 353)
>   6  Parm  === 3  [[ 21 ]] I_O !jvms: NoCompilationOutput::badPhase2 @ bci:-1 (line 353)
>   7  Parm  === 3  [[ 21 ]] Memory  Memory: @BotPTR *+bot, idx=Bot; !jvms: NoCompilationOutput::badPhase2 @ bci:-1 (line 353)
>   8  Parm  === 3  [[ 21 ]] FramePtr !jvms: NoCompilationOutput::badPhase2 @ bci:-1 (line 353)
>   9  Parm  === 3  [[ 21 ]] ReturnAdr !jvms: NoCompilationOutput::badPhase2 @ bci:-1 (line 353)
>  21  Return  === 5 6 7 8 9  [[ 0 ]]
> 
> which is not surprising given this method has an empty body.
> 
> What is surprising is that `ITER_GVN_AFTER_ELIMINATION` gives some output, which is what the test is about (cf. `@ExpectedFailure(ruleId = 1, hasCompilation = false,`).
> 
> So, if we just remove this phase in the test, we are good... as long as it's legitimate to do so. And I think it is: #1447 added some macro expansions (including an IGVN round with phase `PHASE_ITER_GVN_AFTER_ELIMINATION`) to `Compile::Optimize` just before the escape analysis, one of those being unconditional:
> https://github.com/openjdk/valhalla/blob/7e4c1bc1753bcd9655214eca32c391d28af43267/src/hotspot/share/opto/compile.cpp#L2858-L2867
> And this code being Valhalla-specific, that explains why our test is failing only on Valhalla.
> 
> So the fix is just as simple as not trying to find a failure for this phase.
> 
> I think it's very fine because this test is more about testing IR framework features than making sure this particular phase is missing on this method.
> 
> ===
> 
> After review comment, let's change solution: the diagnostic is still valid, but instead, let's run macro expansion only when there are macro nodes. In our case, that means not to run it. The test needs no more fixing.
> 
> Thanks,
> Marc

The second new `if` is not required for the test to pass, but it seems more consistent to have it as well.

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

PR Comment: https://git.openjdk.org/valhalla/pull/1508#issuecomment-3072829796