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
--- 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);