RFR: 8305073: Fix VerifyLoopOptimizations - step 2 - verify idom

[Emanuel Peter]

On Mon, 15 May 2023 07:43:27 GMT, Roberto Castañeda Lozano <rcastanedalo at openjdk.org> wrote:

>> This is the second step in the `VerifyLoopOptimizations` revival.
>> 
>> Last step:
>> [JDK-8173709](https://bugs.openjdk.org/browse/JDK-8173709) Fix VerifyLoopOptimizations - step 1 - minimal infrastructure 
>> See PR for all the planned steps: https://github.com/openjdk/jdk/pull/13207
>> 
>> Next step:
>> [JDK-8307982](https://bugs.openjdk.org/browse/JDK-8307982) Fix VerifyLoopOptimizations - step 3 - fix ctrl/loop
>> 
>> -------
>> 
>> There only seemed to be one bug with idom that I could find up to **tier6 and stress testing**. That one bug already showed up with a simple `java -Xcomp --version`. But it is possible that there are more that we would find in the future, maybe with the fuzzer.
>> 
>> **Details about the bug** I fixed in `PhaseIdealLoop::create_new_if_for_predicate`:
>> We computed the `dom_lca_internal` for `rgn` too early - the following line can change the CFG such that the idom would change:
>> https://github.com/openjdk/jdk/blob/1e1abc4c086298060ccb13b63f646a298bbe3ef7/src/hotspot/share/opto/loopPredicate.cpp#L216
>> 
>> So I moved the idom computation down, until after we do not change the CFG anymore, and idom should be stable from there on.
>
> Hi Emanuel, thanks for your useful work in resurrecting `VerifyLoopOptimizations`!
> I think the issue in `loopPredicate.cpp` should be reported (as a bug) and addressed separately, ideally (if feasible) with a corresponding test case.

@robcasloz I think this is very difficult, if it is even possible at all. The idom info may be wrong, but I'm not sure if it ever leads to a real failure. Honestly, there are too many idom / ctrl / loop "bugs" that I will encounter with this verification work. If we are worried about backports, we can split the fixes from the verification. But finding regression tests that actually mainfest in an assert or wrong execution, that will be an immense effort.

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

PR Comment: https://git.openjdk.org/jdk/pull/13951#issuecomment-1547408531