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

[Roberto Castañeda Lozano]

On Tue, 15 Dec 2020 11:05:36 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.

This pull request has now been integrated.

Changeset: 4e8338eb
Author:    Roberto Castañeda Lozano <rcastanedalo at openjdk.org>
Committer: Christian Hagedorn <chagedorn at openjdk.org>
URL:       https://git.openjdk.java.net/jdk16/commit/4e8338eb
Stats:     161 lines in 4 files changed: 157 ins; 0 del; 4 mod

8255763: C2: OSR miscompilation caused by invalid memory instruction placement

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.

Reviewed-by: kvn, chagedorn

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

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