Integrated: 8261730: C2 compilation fails with assert(store->find_edge(load) != -1) failed: missing precedence edge

[Roberto Castañeda Lozano]

On Mon, 1 Mar 2021 09:53:02 GMT, Roberto Castañeda Lozano <rcastanedalo at openjdk.org> wrote:

> This change relaxes an assertion in `PhaseCFG::verify()` to accept the case where a store is used to implement an implicit null check and a load is placed in the null block.
> 
> The implicit null check transformation `PhaseCFG::implicit_null_check()` tries to reuse globally scheduled memory operations to perform null checks, hoisting the memory operations into null-check blocks if necessary. When hoisting stores, `implicit_null_check()` preserves anti-dependences between the hoisted store and loads in the corresponding null-check and not-null blocks, but disregards possible anti-dependences with loads in the null block. This is safe, as the null block is only executed when the hoisted store has "failed" (attempted to write to the null address). This change makes `PhaseCFG::verify()` aware of this fact to avoid falsely reporting an anti-dependence violation (missing precedence edge from the load in the null block to the store hoisted to the null-check block).
> 
> **Example.** After global scheduling ([before-implicit-null-check.pdf](https://github.com/openjdk/jdk/files/6060581/before-implicit-null-check.pdf)), `implicit_null_check()` detects a store `(17 storeImmIO)` in B6 that could potentially perform the null check done by `(5 testN_reg)` in B4. After checking that neither B4 or B6 contain anti-dependent loads, the transformation replaces `(5 testN_reg)` with `(17 storeImmIO)` by hoisting the latter into B4 and adding an additional `(46 NullCheck)` pseudo-operation ([after-implicit-null-check.pdf](https://github.com/openjdk/jdk/files/6060614/after-implicit-null-check.pdf)). Finally, after local scheduling, `PhaseCFG::verify()` falsely reports that `(17 storeImmIO)` can overwrite the memory value read by the load operation `(34 subI_rReg_mem)` in the null block B5. This overwrite cannot happen, because executing B5 implies `(17 storeImmIO)`'s failure to store any value.
> 
> Tested on hs-tier1-5 (windows-x64, linux-x64, linux-aarch64, and macosx-x64).

This pull request has now been integrated.

Changeset: 4cfecceb
Author:    Roberto Castañeda Lozano <rcastanedalo at openjdk.org>
URL:       https://git.openjdk.java.net/jdk/commit/4cfecceb
Stats:     82 lines in 4 files changed: 80 ins; 0 del; 2 mod

8261730: C2 compilation fails with assert(store->find_edge(load) != -1) failed: missing precedence edge

Relax assertion in PhaseCFG::verify() to accept the case where a store is used
to implement an implicit null check and a load is placed in the null block.

Reviewed-by: thartmann, kvn

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

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