[jdk16] RFR: 8255763: C2: OSR miscompilation caused by invalid memory instruction placement

[Roberto Castañeda Lozano]

On Tue, 15 Dec 2020 11:14:59 GMT, Roberto Castañeda Lozano <rcastanedalo at openjdk.org> wrote:

>> Disable GCM hoisting of memory-writing nodes for irreducible CFGs. This prevents
>> GCM from wrongly "hoisting" stores into descendants of their original loop. Such
>> an "inverted hoisting" can happen due to CFGLoop::compute_freq()'s inaccurate
>> estimation of frequencies for irreducible CFGs.
>> 
>> Extend CFG verification code by checking that memory-writing nodes are placed in
>> either their original loop or an ancestor.
>> 
>> Add tests for the reducible and irreducible cases. The former was already
>> handled correctly before the change (the frequency estimation model prevents
>> "inverted hoisting" for reducible CFGs), and is just added for coverage.
>> 
>> This change addresses the specific miscompilation issue in a conservative way,
>> for simplicity and safety. Future work includes investigating if only the
>> illegal blocks can be discarded as candidates for GCM hoisting, and refining
>> frequency estimation for irreducible CFGs.
>
> Tested on `hs-tier1-9` on windows-x64, linux-x64, linux-aarch64, and macosx-x64 with `VerifyRegisterAllocator` enabled (to exercise all calls to the updated `PhaseCFG::verify()`).

Tested for performance regressions on a set of standard benchmark suites (DaCapo, SPECjbb2005, SPECjvm2008, ...) and on windows-x64, linux-x64, and macosx-x64. No regression was observed, which can be expected since this change 1) only affects a minority of the compiled methods (those with irreducible CFGs), 2) only affects the placement of memory-writing nodes, which tends to be quite constrained already, and 3) forces the placement of these nodes, as much as possible, out of loops.

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

PR: https://git.openjdk.java.net/jdk16/pull/22