8265132: C2 compilation fails with assert "missing precedence edge"

Reviewed-by: iveresov, kvn

src/hotspot/share/opto/block.hpp

@@ -625,6 +625,8 @@ class PhaseCFG : public Phase {
   bool trace_opto_pipelining() const { return false; }
 #endif
 
+  bool unrelated_load_in_store_null_block(Node* store, Node* load);
+
   // Check that block b is in the home loop (or an ancestor) of n, if n is a
   // memory writer.
   void verify_memory_writer_placement(const Block* b, const Node* n) const NOT_DEBUG_RETURN;

src/hotspot/share/opto/gcm.cpp

@@ -540,6 +540,28 @@ static Block* memory_early_block(Node* load, Block* early, const PhaseCFG* cfg)
   return early;
 }
 
+// This function is used by insert_anti_dependences to find unrelated loads for stores in implicit null checks.
+bool PhaseCFG::unrelated_load_in_store_null_block(Node* store, Node* load) {
+  // We expect an anti-dependence edge from 'load' to 'store', except when
+  // implicit_null_check() has hoisted 'store' above its early block to
+  // perform an implicit null check, and 'load' is placed in the null
+  // block. In this case it is safe to ignore the anti-dependence, as the
+  // null block is only reached if 'store' tries to write to null object and
+  // 'load' read from non-null object (there is preceding check for that)
+  // These objects can't be the same.
+  Block* store_block = get_block_for_node(store);
+  Block* load_block = get_block_for_node(load);
+  Node* end = store_block->end();
+  if (end->is_MachNullCheck() && (end->in(1) == store) && store_block->dominates(load_block)) {
+    Node* if_true = end->find_out_with(Op_IfTrue);
+    assert(if_true != NULL, "null check without null projection");
+    Node* null_block_region = if_true->find_out_with(Op_Region);
+    assert(null_block_region != NULL, "null check without null region");
+    return get_block_for_node(null_block_region) == load_block;
+  }
+  return false;
+}
+
 //--------------------------insert_anti_dependences---------------------------
 // A load may need to witness memory that nearby stores can overwrite.
 // For each nearby store, either insert an "anti-dependence" edge
@@ -759,7 +781,7 @@ Block* PhaseCFG::insert_anti_dependences(Block* LCA, Node* load, bool verify) {
       // will find him on the non_early_stores list and stick him
       // with a precedence edge.
       // (But, don't bother if LCA is already raised all the way.)
-      if (LCA != early) {
+      if (LCA != early && !unrelated_load_in_store_null_block(store, load)) {
         store_block->set_raise_LCA_mark(load_index);
         must_raise_LCA = true;
         non_early_stores.push(store);
@@ -770,23 +792,7 @@ Block* PhaseCFG::insert_anti_dependences(Block* LCA, Node* load, bool verify) {
       // Add an anti-dep edge, and squeeze 'load' into the highest block.
       assert(store != load->find_exact_control(load->in(0)), "dependence cycle found");
       if (verify) {
-#ifdef ASSERT
-        // We expect an anti-dependence edge from 'load' to 'store', except when
-        // implicit_null_check() has hoisted 'store' above its early block to
-        // perform an implicit null check, and 'load' is placed in the null
-        // block. In this case it is safe to ignore the anti-dependence, as the
-        // null block is only reached if 'store' tries to write to null.
-        Block* store_null_block = NULL;
-        Node* store_null_check = store->find_out_with(Op_MachNullCheck);
-        if (store_null_check != NULL) {
-          Node* if_true = store_null_check->find_out_with(Op_IfTrue);
-          assert(if_true != NULL, "null check without null projection");
-          Node* null_block_region = if_true->find_out_with(Op_Region);
-          assert(null_block_region != NULL, "null check without null region");
-          store_null_block = get_block_for_node(null_block_region);
-        }
-#endif
-        assert(LCA == store_null_block || store->find_edge(load) != -1,
+        assert(store->find_edge(load) != -1 || unrelated_load_in_store_null_block(store, load),
                "missing precedence edge");
       } else {
         store->add_prec(load);

test/hotspot/jtreg/compiler/uncommontrap/TestNullCheckAntiDependence.java

@@ -25,7 +25,7 @@ package compiler.uncommontrap;
 
 /**
  * @test
- * @bug 8261730
+ * @bug 8261730 8265132
  * @summary Test that no anti-dependence violation is reported between a store
  *          used as an implicit null check and a load placed in the null block.
  * @run main/othervm -XX:-BackgroundCompilation
@@ -40,8 +40,9 @@ public class TestNullCheckAntiDependence {
 
     private static MyInteger foo = new MyInteger();
     private static MyInteger bar = new MyInteger();
+    private static MyInteger[] global = {new MyInteger()};
 
-    static void setFooToZero() {
+    static void test1() {
         for (int i = 0; i < 1; i++) {
             // This load is placed in the null block.
             foo.val = -bar.val;
@@ -52,10 +53,21 @@ public class TestNullCheckAntiDependence {
         }
     }
 
+    static void test2(MyInteger a, MyInteger b) {
+        global[0].val = a.val + b.val * 31;
+        global[0].val = 0;
+        return;
+    }
+
     public static void main(String[] args) {
         for (int i = 0; i < 10_000; i++) {
-            setFooToZero();
+            test1();
         }
+
+        for (int i = 0; i < 10_000; i++) {
+            test2(new MyInteger(), new MyInteger());
+        }
+
     }
 
 }