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