RFR: 8258894: C2: Forbid GCM to move stores into loops

Roberto Castañeda Lozano rcastanedalo at openjdk.java.net
Tue Jan 26 08:30:39 UTC 2021


On Mon, 25 Jan 2021 08:48:01 GMT, Tobias Hartmann <thartmann at openjdk.org> wrote:

> 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?

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

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


More information about the hotspot-compiler-dev mailing list