# HG changeset patch # User roland # Date 1442315323 -7200 # Node ID f6b082ab565eb028e08242b76720ee6e4c2142ba # Parent 69a83b5ce3903ebbe357e76a911f4244b3cac942 8136461: PhaseIdealLoop::try_move_store_before_loop() may bypass early loop exit Summary: PhaseIdealLoop::try_move_store_before_loop() needs to check for early loop exit before candidate Stores Reviewed-by: kvn diff -r 69a83b5ce390 -r f6b082ab565e hotspot/src/share/vm/opto/loopopts.cpp --- a/hotspot/src/share/vm/opto/loopopts.cpp Tue Sep 15 12:59:51 2015 +0000 +++ b/hotspot/src/share/vm/opto/loopopts.cpp Tue Sep 15 13:08:43 2015 +0200 @@ -653,7 +653,6 @@ return iff->in(1); } -#ifdef ASSERT static void enqueue_cfg_uses(Node* m, Unique_Node_List& wq) { for (DUIterator_Fast imax, i = m->fast_outs(imax); i < imax; i++) { Node* u = m->fast_out(i); @@ -667,7 +666,6 @@ } } } -#endif // Try moving a store out of a loop, right before the loop Node* PhaseIdealLoop::try_move_store_before_loop(Node* n, Node *n_ctrl) { @@ -687,11 +685,15 @@ // written at iteration i by the second store could be overwritten // at iteration i+n by the first store: it's not safe to move the // first store out of the loop - // - nothing must observe the Phi memory: it guarantees no read - // before the store and no early exit out of the loop - // With those conditions, we are also guaranteed the store post - // dominates the loop head. Otherwise there would be extra Phi - // involved between the loop's Phi and the store. + // - nothing must observe the memory Phi: it guarantees no read + // before the store, we are also guaranteed the store post + // dominates the loop head (ignoring a possible early + // exit). Otherwise there would be extra Phi involved between the + // loop's Phi and the store. + // - there must be no early exit from the loop before the Store + // (such an exit most of the time would be an extra use of the + // memory Phi but sometimes is a bottom memory Phi that takes the + // store as input). if (!n_loop->is_member(address_loop) && !n_loop->is_member(value_loop) && @@ -699,9 +701,10 @@ mem->outcnt() == 1 && mem->in(LoopNode::LoopBackControl) == n) { -#ifdef ASSERT - // Verify that store's control does post dominate loop entry and - // that there's no early exit of the loop before the store. + assert(n_loop->_tail != NULL, "need a tail"); + assert(is_dominator(n_ctrl, n_loop->_tail), "store control must not be in a branch in the loop"); + + // Verify that there's no early exit of the loop before the store. bool ctrl_ok = false; { // Follow control from loop head until n, we exit the loop or @@ -709,7 +712,7 @@ ResourceMark rm; Unique_Node_List wq; wq.push(n_loop->_head); - assert(n_loop->_tail != NULL, "need a tail"); + for (uint next = 0; next < wq.size(); ++next) { Node *m = wq.at(next); if (m == n->in(0)) { @@ -722,24 +725,27 @@ break; } enqueue_cfg_uses(m, wq); + if (wq.size() > 10) { + ctrl_ok = false; + break; + } } } - assert(ctrl_ok, "bad control"); -#endif + if (ctrl_ok) { + // move the Store + _igvn.replace_input_of(mem, LoopNode::LoopBackControl, mem); + _igvn.replace_input_of(n, 0, n_loop->_head->in(LoopNode::EntryControl)); + _igvn.replace_input_of(n, MemNode::Memory, mem->in(LoopNode::EntryControl)); + // Disconnect the phi now. An empty phi can confuse other + // optimizations in this pass of loop opts. + _igvn.replace_node(mem, mem->in(LoopNode::EntryControl)); + n_loop->_body.yank(mem); - // move the Store - _igvn.replace_input_of(mem, LoopNode::LoopBackControl, mem); - _igvn.replace_input_of(n, 0, n_loop->_head->in(LoopNode::EntryControl)); - _igvn.replace_input_of(n, MemNode::Memory, mem->in(LoopNode::EntryControl)); - // Disconnect the phi now. An empty phi can confuse other - // optimizations in this pass of loop opts. - _igvn.replace_node(mem, mem->in(LoopNode::EntryControl)); - n_loop->_body.yank(mem); + IdealLoopTree* new_loop = get_loop(n->in(0)); + set_ctrl_and_loop(n, n->in(0)); - IdealLoopTree* new_loop = get_loop(n->in(0)); - set_ctrl_and_loop(n, n->in(0)); - - return n; + return n; + } } } return NULL; diff -r 69a83b5ce390 -r f6b082ab565e hotspot/test/compiler/TestMoveStoresOutOfLoopsStoreNoCtrl.java --- a/hotspot/test/compiler/TestMoveStoresOutOfLoopsStoreNoCtrl.java Tue Sep 15 12:59:51 2015 +0000 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,49 +0,0 @@ -/* - * Copyright (c) 2015, 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 8134288 - * @summary Store nodes may not have a control if used to update profiling - * @run main/othervm -XX:-ProfileInterpreter -XX:-TieredCompilation -XX:-BackgroundCompilation TestMoveStoresOutOfLoopsStoreNoCtrl - * - */ - -public class TestMoveStoresOutOfLoopsStoreNoCtrl { - - static void test(boolean flag) { - for (int i = 0; i < 20000; i++) { - if (flag) { - int j = 0; - do { - j++; - } while(j < 10); - } - } - } - - static public void main(String[] args) { - test(false); - } - -} diff -r 69a83b5ce390 -r f6b082ab565e hotspot/test/compiler/loopopts/TestMoveStoresOutOfLoopsStoreNoCtrl.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/hotspot/test/compiler/loopopts/TestMoveStoresOutOfLoopsStoreNoCtrl.java Tue Sep 15 13:08:43 2015 +0200 @@ -0,0 +1,49 @@ +/* + * Copyright (c) 2015, 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 8134288 + * @summary Store nodes may not have a control if used to update profiling + * @run main/othervm -XX:-ProfileInterpreter -XX:-TieredCompilation -XX:-BackgroundCompilation TestMoveStoresOutOfLoopsStoreNoCtrl + * + */ + +public class TestMoveStoresOutOfLoopsStoreNoCtrl { + + static void test(boolean flag) { + for (int i = 0; i < 20000; i++) { + if (flag) { + int j = 0; + do { + j++; + } while(j < 10); + } + } + } + + static public void main(String[] args) { + test(false); + } + +}