RFR: 8258894: C2: Forbid GCM to move stores into loops [v2]

[Roberto Castañeda Lozano]

On Tue, 26 Jan 2021 08:27:37 GMT, Roberto Castañeda Lozano <rcastanedalo at openjdk.org> wrote:

>> Looks good to me. Just to clarify, with JDK-8255763 in place, incorrect placement in the reducible CFG case is always prevented by the frequency estimation heuristic, right? I.e. without this patch, your newly added tests wouldn't fail? Would it make sense to run the test with `StressGCM` to trigger actual failures?
>
>> Looks good to me.
> 
> Thanks for reviewing, Tobias!
> 
>> Just to clarify, with JDK-8255763 in place, incorrect placement in the reducible CFG case is always prevented by the frequency estimation heuristic, right?
> 
> Even for reducible CFGs there are cases (e.g. `testRegularReducible[2|3|4]()`) where GCM moves stores into loops deeper than their home loop, causing memory definitions to interfere _statically_: this is what I refer to as "invalid". However, in all these cases, the inner loop into which the store is moved has a trip count of one (otherwise its estimated frequency would be higher and the store would not be placed there), so there is no _dynamic_ interference and hence no possible miscompilation.
> 
>> I.e. without this patch, your newly added tests wouldn't fail?
> 
> Right, no assertion would fail (because C2 does not verify memory liveness after GCM), and no miscompilation would happen.  
> 
>> Would it make sense to run the test with `StressGCM` to trigger actual failures?
> 
> Yes, but triggering failures would require [JDK-8257146](https://bugs.openjdk.java.net/browse/JDK-8257146), which is blocked by this issue, to be integrated. I propose to add `StressGCM` to the tests later in JDK-8257146. Does that make sense?

I just applied the refactoring suggested by Tobias (commit d8ff541), re-tested on hs-tier1 (windows-x64, linux-x64, linux-aarch64, and macosx-x64; release and debug mode), and checked manually that the refactored verification code is executed in debug mode.

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

PR: https://git.openjdk.java.net/jdk/pull/2140