8189067: SuperWord optimization crashes with "assert(out == prev || prev == __null) failed: no branches off of store slice"
authorthartmann
Fri, 13 Oct 2017 14:02:01 +0200
changeset 47623 0a5f1b851890
parent 47622 817f2a7019e4
child 47624 b055cb5170f5
8189067: SuperWord optimization crashes with "assert(out == prev || prev == __null) failed: no branches off of store slice" Summary: Only move the store if the LCA of all users is outside of the loop. Re-enable UseSubwordForMaxVector. Reviewed-by: kvn, roland
src/hotspot/share/opto/c2_globals.hpp
src/hotspot/share/opto/loopopts.cpp
test/hotspot/jtreg/compiler/loopopts/TestMoveStoresOutOfLoops.java
--- a/src/hotspot/share/opto/c2_globals.hpp	Thu Oct 12 15:08:19 2017 +0200
+++ b/src/hotspot/share/opto/c2_globals.hpp	Fri Oct 13 14:02:01 2017 +0200
@@ -192,7 +192,7 @@
           "of rounds of unroll,optimize,..")                                \
           range(0, max_jint)                                                \
                                                                             \
-  product(bool, UseSubwordForMaxVector, false,                              \
+  product(bool, UseSubwordForMaxVector, true,                               \
           "Use Subword Analysis to set maximum vector size")                \
                                                                             \
   develop(intx, UnrollLimitForProfileCheck, 1,                              \
--- a/src/hotspot/share/opto/loopopts.cpp	Thu Oct 12 15:08:19 2017 +0200
+++ b/src/hotspot/share/opto/loopopts.cpp	Fri Oct 13 14:02:01 2017 +0200
@@ -826,45 +826,26 @@
             }
           }
           if (mem_ok) {
-            // Move the Store out of the loop creating clones along
-            // all paths out of the loop that observe the stored value
+            // Move the store out of the loop if the LCA of all
+            // users (except for the phi) is outside the loop.
+            Node* hook = new Node(1);
             _igvn.rehash_node_delayed(phi);
-            int count = phi->replace_edge(n, n->in(MemNode::Memory));
+            int count = phi->replace_edge(n, hook);
             assert(count > 0, "inconsistent phi");
-            for (DUIterator_Fast imax, i = n->fast_outs(imax); i < imax; i++) {
-              Node* u = n->fast_out(i);
-              Node* c = get_ctrl(u);
 
-              if (u->is_Phi()) {
-                c = u->in(0)->in(u->find_edge(n));
-              }
-              IdealLoopTree *u_loop = get_loop(c);
-              assert (!n_loop->is_member(u_loop), "only the phi should have been a use in the loop");
-              while(true) {
-                Node* next_c = find_non_split_ctrl(idom(c));
-                if (n_loop->is_member(get_loop(next_c))) {
-                  break;
-                }
-                c = next_c;
-              }
-
-              Node* st = n->clone();
-              st->set_req(0, c);
-              _igvn.register_new_node_with_optimizer(st);
-
-              set_ctrl(st, c);
-              IdealLoopTree* new_loop = get_loop(c);
-              assert(new_loop != n_loop, "should be moved out of loop");
-              if (new_loop->_child == NULL) new_loop->_body.push(st);
-
-              _igvn.replace_input_of(u, u->find_edge(n), st);
-              --imax;
-              --i;
+            // Compute latest point this store can go
+            Node* lca = get_late_ctrl(n, get_ctrl(n));
+            if (n_loop->is_member(get_loop(lca))) {
+              // LCA is in the loop - bail out
+              _igvn.replace_node(hook, n);
+              return;
             }
 
+            // Move store out of the loop
+            _igvn.replace_node(hook, n->in(MemNode::Memory));
+            _igvn.replace_input_of(n, 0, lca);
+            set_ctrl_and_loop(n, lca);
 
-            assert(n->outcnt() == 0, "all uses should be gone");
-            _igvn.replace_input_of(n, MemNode::Memory, C->top());
             // Disconnect the phi now. An empty phi can confuse other
             // optimizations in this pass of loop opts..
             if (phi->in(LoopNode::LoopBackControl) == phi) {
--- a/test/hotspot/jtreg/compiler/loopopts/TestMoveStoresOutOfLoops.java	Thu Oct 12 15:08:19 2017 +0200
+++ b/test/hotspot/jtreg/compiler/loopopts/TestMoveStoresOutOfLoops.java	Fri Oct 13 14:02:01 2017 +0200
@@ -23,7 +23,7 @@
 
 /**
  * @test
- * @bug 8080289
+ * @bug 8080289 8189067
  * @summary Move stores out of loops if possible
  *
  * @run main/othervm -XX:-UseOnStackReplacement -XX:-BackgroundCompilation
@@ -43,6 +43,7 @@
     private static long[] array = new long[10];
     private static long[] array2 = new long[10];
     private static boolean[] array3 = new boolean[1000];
+    private static int[] array4 = new int[1000];
     private static byte[] byte_array = new byte[10];
 
     // Array store should be moved out of the loop, value stored
@@ -108,6 +109,15 @@
         }
     }
 
+    // Array store can be moved out of the inner loop
+    static void test_after_7(int idx) {
+        for (int i = 0; i < 1000; i++) {
+            for (int j = 0; j <= 42; j++) {
+                array4[i] = j;
+            }
+        }
+    }
+
     // Optimize out redundant stores
     static void test_stores_1(int ignored) {
         array[0] = 0;
@@ -285,6 +295,17 @@
         return success;
     }
 
+    static boolean array_check5(String name) {
+        boolean success = true;
+        for (int i = 0; i < 1000; i++) {
+            if (array4[i] != 42) {
+                success = false;
+                System.out.println(name + " failed: array[" + i + "] = " + array4[i]);
+            }
+        }
+        return success;
+    }
+
     static public void main(String[] args) throws Exception {
         TestMoveStoresOutOfLoops test = new TestMoveStoresOutOfLoops();
         test.doTest("test_after_1", TestMoveStoresOutOfLoops::array_init, TestMoveStoresOutOfLoops::array_check);
@@ -295,6 +316,7 @@
         test.doTest("test_after_6", TestMoveStoresOutOfLoops::array_init, TestMoveStoresOutOfLoops::array_check);
         array3[999] = true;
         test.doTest("test_after_6", TestMoveStoresOutOfLoops::array_init, TestMoveStoresOutOfLoops::array_check);
+        test.doTest("test_after_7", TestMoveStoresOutOfLoops::array_init, TestMoveStoresOutOfLoops::array_check5);
 
         test.doTest("test_stores_1", TestMoveStoresOutOfLoops::array_init3, TestMoveStoresOutOfLoops::array_check3);
         test.doTest("test_stores_2", TestMoveStoresOutOfLoops::array_init3, TestMoveStoresOutOfLoops::array_check3);