# HG changeset patch # User kvn # Date 1309299869 25200 # Node ID efb61be7eaa751cead3c7406f2dba2d79f6418b8 # Parent 2abb145bee7edcc76a8d85a10508ad052c5e8a6b 7044738: Loop unroll optimization causes incorrect result Summary: take into account memory dependencies when clonning nodes in clone_up_backedge_goo(). Reviewed-by: never diff -r 2abb145bee7e -r efb61be7eaa7 src/share/vm/opto/loopTransform.cpp --- a/src/share/vm/opto/loopTransform.cpp Tue Jul 26 19:35:23 2011 -0700 +++ b/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 ); } diff -r 2abb145bee7e -r efb61be7eaa7 src/share/vm/opto/loopnode.hpp --- a/src/share/vm/opto/loopnode.hpp Tue Jul 26 19:35:23 2011 -0700 +++ b/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 diff -r 2abb145bee7e -r efb61be7eaa7 src/share/vm/opto/macro.cpp --- a/src/share/vm/opto/macro.cpp Tue Jul 26 19:35:23 2011 -0700 +++ b/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 diff -r 2abb145bee7e -r efb61be7eaa7 src/share/vm/opto/node.cpp --- a/src/share/vm/opto/node.cpp Tue Jul 26 19:35:23 2011 -0700 +++ b/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 diff -r 2abb145bee7e -r efb61be7eaa7 src/share/vm/opto/node.hpp --- a/src/share/vm/opto/node.hpp Tue Jul 26 19:35:23 2011 -0700 +++ b/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; }; diff -r 2abb145bee7e -r efb61be7eaa7 test/compiler/7044738/Test7044738.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/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]; + } + +} diff -r 2abb145bee7e -r efb61be7eaa7 test/compiler/7046096/Test7046096.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/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); + } + } + } +} +