7044738: Loop unroll optimization causes incorrect result
Summary: take into account memory dependencies when clonning nodes in clone_up_backedge_goo().
Reviewed-by: never
--- a/hotspot/src/share/vm/opto/loopTransform.cpp Tue Jun 28 15:04:39 2011 -0700
+++ b/hotspot/src/share/vm/opto/loopTransform.cpp Tue Jun 28 15:24:29 2011 -0700
@@ -824,13 +824,23 @@
//------------------------------clone_up_backedge_goo--------------------------
// If Node n lives in the back_ctrl block and cannot float, we clone a private
// version of n in preheader_ctrl block and return that, otherwise return n.
-Node *PhaseIdealLoop::clone_up_backedge_goo( Node *back_ctrl, Node *preheader_ctrl, Node *n ) {
+Node *PhaseIdealLoop::clone_up_backedge_goo( Node *back_ctrl, Node *preheader_ctrl, Node *n, VectorSet &visited, Node_Stack &clones ) {
if( get_ctrl(n) != back_ctrl ) return n;
+ // Only visit once
+ if (visited.test_set(n->_idx)) {
+ Node *x = clones.find(n->_idx);
+ if (x != NULL)
+ return x;
+ return n;
+ }
+
Node *x = NULL; // If required, a clone of 'n'
// Check for 'n' being pinned in the backedge.
if( n->in(0) && n->in(0) == back_ctrl ) {
+ assert(clones.find(n->_idx) == NULL, "dead loop");
x = n->clone(); // Clone a copy of 'n' to preheader
+ clones.push(x, n->_idx);
x->set_req( 0, preheader_ctrl ); // Fix x's control input to preheader
}
@@ -838,10 +848,13 @@
// If there are no changes we can just return 'n', otherwise
// we need to clone a private copy and change it.
for( uint i = 1; i < n->req(); i++ ) {
- Node *g = clone_up_backedge_goo( back_ctrl, preheader_ctrl, n->in(i) );
+ Node *g = clone_up_backedge_goo( back_ctrl, preheader_ctrl, n->in(i), visited, clones );
if( g != n->in(i) ) {
- if( !x )
+ if( !x ) {
+ assert(clones.find(n->_idx) == NULL, "dead loop");
x = n->clone();
+ clones.push(x, n->_idx);
+ }
x->set_req(i, g);
}
}
@@ -960,6 +973,9 @@
post_head->set_req(LoopNode::EntryControl, zer_taken);
set_idom(post_head, zer_taken, dd_main_exit);
+ Arena *a = Thread::current()->resource_area();
+ VectorSet visited(a);
+ Node_Stack clones(a, main_head->back_control()->outcnt());
// Step A3: Make the fall-in values to the post-loop come from the
// fall-out values of the main-loop.
for (DUIterator_Fast imax, i = main_head->fast_outs(imax); i < imax; i++) {
@@ -968,7 +984,8 @@
Node *post_phi = old_new[main_phi->_idx];
Node *fallmain = clone_up_backedge_goo(main_head->back_control(),
post_head->init_control(),
- main_phi->in(LoopNode::LoopBackControl));
+ main_phi->in(LoopNode::LoopBackControl),
+ visited, clones);
_igvn.hash_delete(post_phi);
post_phi->set_req( LoopNode::EntryControl, fallmain );
}
@@ -1032,6 +1049,8 @@
main_head->set_req(LoopNode::EntryControl, min_taken);
set_idom(main_head, min_taken, dd_main_head);
+ visited.Clear();
+ clones.clear();
// Step B3: Make the fall-in values to the main-loop come from the
// fall-out values of the pre-loop.
for (DUIterator_Fast i2max, i2 = main_head->fast_outs(i2max); i2 < i2max; i2++) {
@@ -1040,7 +1059,8 @@
Node *pre_phi = old_new[main_phi->_idx];
Node *fallpre = clone_up_backedge_goo(pre_head->back_control(),
main_head->init_control(),
- pre_phi->in(LoopNode::LoopBackControl));
+ pre_phi->in(LoopNode::LoopBackControl),
+ visited, clones);
_igvn.hash_delete(main_phi);
main_phi->set_req( LoopNode::EntryControl, fallpre );
}
--- a/hotspot/src/share/vm/opto/loopnode.hpp Tue Jun 28 15:04:39 2011 -0700
+++ b/hotspot/src/share/vm/opto/loopnode.hpp Tue Jun 28 15:24:29 2011 -0700
@@ -843,7 +843,7 @@
void insert_pre_post_loops( IdealLoopTree *loop, Node_List &old_new, bool peel_only );
// If Node n lives in the back_ctrl block, we clone a private version of n
// in preheader_ctrl block and return that, otherwise return n.
- Node *clone_up_backedge_goo( Node *back_ctrl, Node *preheader_ctrl, Node *n );
+ Node *clone_up_backedge_goo( Node *back_ctrl, Node *preheader_ctrl, Node *n, VectorSet &visited, Node_Stack &clones );
// Take steps to maximally unroll the loop. Peel any odd iterations, then
// unroll to do double iterations. The next round of major loop transforms
--- a/hotspot/src/share/vm/opto/macro.cpp Tue Jun 28 15:04:39 2011 -0700
+++ b/hotspot/src/share/vm/opto/macro.cpp Tue Jun 28 15:24:29 2011 -0700
@@ -391,13 +391,9 @@
}
}
// Check if an appropriate new value phi already exists.
- Node* new_phi = NULL;
- uint size = value_phis->size();
- for (uint i=0; i < size; i++) {
- if ( mem->_idx == value_phis->index_at(i) ) {
- return value_phis->node_at(i);
- }
- }
+ Node* new_phi = value_phis->find(mem->_idx);
+ if (new_phi != NULL)
+ return new_phi;
if (level <= 0) {
return NULL; // Give up: phi tree too deep
--- a/hotspot/src/share/vm/opto/node.cpp Tue Jun 28 15:04:39 2011 -0700
+++ b/hotspot/src/share/vm/opto/node.cpp Tue Jun 28 15:24:29 2011 -0700
@@ -2012,6 +2012,16 @@
_inode_top = _inodes + old_top; // restore _top
}
+// Node_Stack is used to map nodes.
+Node* Node_Stack::find(uint idx) const {
+ uint sz = size();
+ for (uint i=0; i < sz; i++) {
+ if (idx == index_at(i) )
+ return node_at(i);
+ }
+ return NULL;
+}
+
//=============================================================================
uint TypeNode::size_of() const { return sizeof(*this); }
#ifndef PRODUCT
--- a/hotspot/src/share/vm/opto/node.hpp Tue Jun 28 15:04:39 2011 -0700
+++ b/hotspot/src/share/vm/opto/node.hpp Tue Jun 28 15:24:29 2011 -0700
@@ -1463,6 +1463,9 @@
bool is_nonempty() const { return (_inode_top >= _inodes); }
bool is_empty() const { return (_inode_top < _inodes); }
void clear() { _inode_top = _inodes - 1; } // retain storage
+
+ // Node_Stack is used to map nodes.
+ Node* find(uint idx) const;
};
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/hotspot/test/compiler/7044738/Test7044738.java Tue Jun 28 15:24:29 2011 -0700
@@ -0,0 +1,104 @@
+/*
+ * Copyright (c) 2011, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ *
+ */
+
+/**
+ * @test
+ * @bug 7044738
+ * @summary Loop unroll optimization causes incorrect result
+ *
+ * @run main/othervm -Xbatch Test7044738
+ */
+
+public class Test7044738 {
+
+ private static final int INITSIZE = 10000;
+ public int d[] = { 1, 2, 3, 4 };
+ public int i, size;
+
+ private static int iter = 5;
+
+ boolean done() { return (--iter > 0); }
+
+ public static void main(String args[]) {
+ Test7044738 t = new Test7044738();
+ t.test();
+ }
+
+ int test() {
+
+ while (done()) {
+ size = INITSIZE;
+
+ for (i = 0; i < size; i++) {
+ d[0] = d[1]; // 2
+ d[1] = d[2]; // 3
+ d[2] = d[3]; // 4
+ d[3] = d[0]; // 2
+
+ d[0] = d[1]; // 3
+ d[1] = d[2]; // 4
+ d[2] = d[3]; // 2
+ d[3] = d[0]; // 3
+
+ d[0] = d[1]; // 4
+ d[1] = d[2]; // 2
+ d[2] = d[3]; // 3
+ d[3] = d[0]; // 4
+
+ d[0] = d[1]; // 2
+ d[1] = d[2]; // 3
+ d[2] = d[3]; // 4
+ d[3] = d[0]; // 2
+
+ d[0] = d[1]; // 3
+ d[1] = d[2]; // 4
+ d[2] = d[3]; // 2
+ d[3] = d[0]; // 3
+
+ d[0] = d[1]; // 4
+ d[1] = d[2]; // 2
+ d[2] = d[3]; // 3
+ d[3] = d[0]; // 4
+
+ d[0] = d[1]; // 2
+ d[1] = d[2]; // 3
+ d[2] = d[3]; // 4
+ d[3] = d[0]; // 2
+
+ d[0] = d[1]; // 3
+ d[1] = d[2]; // 4
+ d[2] = d[3]; // 2
+ d[3] = d[0]; // 3
+ }
+
+ // try to defeat dead code elimination
+ if (d[0] == d[1]) {
+ System.out.println("test failed: iter=" + iter + " i=" + i + " d[] = { " + d[0] + ", " + d[1] + ", " + d[2] + ", " + d[3] + " } ");
+ System.exit(97);
+ }
+ }
+ return d[3];
+ }
+
+}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/hotspot/test/compiler/7046096/Test7046096.java Tue Jun 28 15:24:29 2011 -0700
@@ -0,0 +1,65 @@
+/*
+ * Copyright (c) 2011, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ *
+ */
+
+/**
+ * @test
+ * @bug 7046096
+ * @summary SEGV IN C2 WITH 6U25
+ *
+ * @run main/othervm -Xbatch -XX:+IgnoreUnrecognizedVMOptions -XX:+OptimizeStringConcat Test7046096
+ */
+
+
+public class Test7046096 {
+
+ static int first = 1;
+
+ String add(String str) {
+ if (first != 0) {
+ return str + "789";
+ } else {
+ return "null";
+ }
+ }
+
+ String test(String str) {
+ for (int i=0; i < first; i++) {
+ if (i > 1)
+ return "bad";
+ }
+ return add(str+"456");
+ }
+
+ public static void main(String [] args) {
+ Test7046096 t = new Test7046096();
+ for (int i = 0; i < 11000; i++) {
+ String str = t.test("123");
+ if (!str.equals("123456789")) {
+ System.out.println("FAILED: " + str + " != \"123456789\"");
+ System.exit(97);
+ }
+ }
+ }
+}
+