diff --git a/src/hotspot/share/opto/loopnode.hpp b/src/hotspot/share/opto/loopnode.hpp index e12792212f..38f619b2ec 100644 --- a/src/hotspot/share/opto/loopnode.hpp +++ b/src/hotspot/share/opto/loopnode.hpp @@ -1378,7 +1378,7 @@ private: bool exceeding_node_budget(uint required = 0) { assert(C->live_nodes() < C->max_node_limit(), "sanity"); uint available = C->max_node_limit() - C->live_nodes(); - return available < required + _nodes_required; + return available < required + _nodes_required + REQUIRE_MIN; } uint require_nodes(uint require, uint minreq = REQUIRE_MIN) { diff --git a/src/hotspot/share/opto/loopopts.cpp b/src/hotspot/share/opto/loopopts.cpp index 10903b1604..765de246b4 100644 --- a/src/hotspot/share/opto/loopopts.cpp +++ b/src/hotspot/share/opto/loopopts.cpp @@ -3159,7 +3159,8 @@ bool PhaseIdealLoop::partial_peel( IdealLoopTree *loop, Node_List &old_new ) { Node_List worklist(area); Node_List sink_list(area); - if (!may_require_nodes(loop->est_loop_clone_sz(2))) { + uint estimate = loop->est_loop_clone_sz(1); + if (exceeding_node_budget(estimate)) { return false; } @@ -3184,8 +3185,7 @@ bool PhaseIdealLoop::partial_peel( IdealLoopTree *loop, Node_List &old_new ) { // Set of non-cfg nodes to peel are those that are control // dependent on the cfg nodes. - uint i; - for(i = 0; i < loop->_body.size(); i++ ) { + for (uint i = 0; i < loop->_body.size(); i++) { Node *n = loop->_body.at(i); Node *n_c = has_ctrl(n) ? get_ctrl(n) : n; if (peel.test(n_c->_idx)) { @@ -3200,7 +3200,7 @@ bool PhaseIdealLoop::partial_peel( IdealLoopTree *loop, Node_List &old_new ) { // Get a post order schedule of nodes in the peel region // Result in right-most operand. - scheduled_nodelist(loop, peel, peel_list ); + scheduled_nodelist(loop, peel, peel_list); assert(is_valid_loop_partition(loop, peel, peel_list, not_peel), "bad partition"); @@ -3220,25 +3220,21 @@ bool PhaseIdealLoop::partial_peel( IdealLoopTree *loop, Node_List &old_new ) { // Evacuate nodes in peel region into the not_peeled region if possible uint new_phi_cnt = 0; uint cloned_for_outside_use = 0; - for (i = 0; i < peel_list.size();) { + for (uint i = 0; i < peel_list.size();) { Node* n = peel_list.at(i); #ifndef PRODUCT if (TracePartialPeeling) n->dump(); #endif bool incr = true; - if ( !n->is_CFG() ) { - - if ( has_use_in_set(n, not_peel) ) { - + if (!n->is_CFG()) { + if (has_use_in_set(n, not_peel)) { // If not used internal to the peeled region, // move "n" from peeled to not_peeled region. - - if ( !has_use_internal_to_set(n, peel, loop) ) { - + if (!has_use_internal_to_set(n, peel, loop)) { // if not pinned and not a load (which maybe anti-dependent on a store) // and not a CMove (Matcher expects only bool->cmove). if (n->in(0) == NULL && !n->is_Load() && !n->is_CMove()) { - cloned_for_outside_use += clone_for_use_outside_loop( loop, n, worklist ); + cloned_for_outside_use += clone_for_use_outside_loop(loop, n, worklist); sink_list.push(n); peel >>= n->_idx; // delete n from peel set. not_peel <<= n->_idx; // add n to not_peel set. @@ -3254,7 +3250,7 @@ bool PhaseIdealLoop::partial_peel( IdealLoopTree *loop, Node_List &old_new ) { } else { // Otherwise check for special def-use cases that span // the peel/not_peel boundary such as bool->if - clone_for_special_use_inside_loop( loop, n, not_peel, sink_list, worklist ); + clone_for_special_use_inside_loop(loop, n, not_peel, sink_list, worklist); new_phi_cnt++; } } @@ -3262,7 +3258,11 @@ bool PhaseIdealLoop::partial_peel( IdealLoopTree *loop, Node_List &old_new ) { if (incr) i++; } - if (new_phi_cnt > old_phi_cnt + PartialPeelNewPhiDelta) { + estimate += cloned_for_outside_use + new_phi_cnt; + bool exceed_node_budget = !may_require_nodes(estimate); + bool exceed_phi_limit = new_phi_cnt > old_phi_cnt + PartialPeelNewPhiDelta; + + if (exceed_node_budget || exceed_phi_limit) { #ifndef PRODUCT if (TracePartialPeeling) { tty->print_cr("\nToo many new phis: %d old %d new cmpi: %c", @@ -3310,7 +3310,7 @@ bool PhaseIdealLoop::partial_peel( IdealLoopTree *loop, Node_List &old_new ) { const uint clone_exit_idx = 1; const uint orig_exit_idx = 2; - assert(is_valid_clone_loop_form( loop, peel_list, orig_exit_idx, clone_exit_idx ), "bad clone loop"); + assert(is_valid_clone_loop_form(loop, peel_list, orig_exit_idx, clone_exit_idx), "bad clone loop"); Node* head_clone = old_new[head->_idx]; LoopNode* new_head_clone = old_new[new_head->_idx]->as_Loop(); @@ -3318,7 +3318,7 @@ bool PhaseIdealLoop::partial_peel( IdealLoopTree *loop, Node_List &old_new ) { // Add phi if "def" node is in peel set and "use" is not - for(i = 0; i < peel_list.size(); i++ ) { + for (uint i = 0; i < peel_list.size(); i++) { Node *def = peel_list.at(i); if (!def->is_CFG()) { for (DUIterator_Fast jmax, j = def->fast_outs(jmax); j < jmax; j++) { @@ -3374,7 +3374,7 @@ bool PhaseIdealLoop::partial_peel( IdealLoopTree *loop, Node_List &old_new ) { // cloned-not_peeled in(0) in(0) // orig-peeled - for(i = 0; i < loop->_body.size(); i++ ) { + for (uint i = 0; i < loop->_body.size(); i++) { Node *n = loop->_body.at(i); if (!n->is_CFG() && n->in(0) != NULL && not_peel.test(n->_idx) && peel.test(n->in(0)->_idx)) { diff --git a/test/hotspot/jtreg/compiler/loopopts/LoopRotateBadNodeBudget.java b/test/hotspot/jtreg/compiler/loopopts/LoopRotateBadNodeBudget.java new file mode 100644 index 0000000000..d52ce86679 --- /dev/null +++ b/test/hotspot/jtreg/compiler/loopopts/LoopRotateBadNodeBudget.java @@ -0,0 +1,81 @@ +/* + * Copyright (c) 2019, 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 8231565 + * @summary Node estimate for loop rotate is not correct/sufficient: + * assert(delta <= 2 * required) failed: Bad node estimate ... + * + * @requires !vm.graal.enabled + * + * @run main/othervm -XX:PartialPeelNewPhiDelta=5 LoopRotateBadNodeBudget + * @run main/othervm -Xbatch -XX:PartialPeelNewPhiDelta=5 LoopRotateBadNodeBudget + * + * @run main/othervm LoopRotateBadNodeBudget + * @run main/othervm -Xbatch LoopRotateBadNodeBudget + * + * NOTE: Test-case seldom manifesting the problem on fast machines. + */ + +public class LoopRotateBadNodeBudget { + + int h; + float j(int a, int b) { + double d = 0.19881; + int c, e[] = new int[9]; + c = 1; + while (++c < 12) + switch ((c % 7 * 5) + 122) { + case 156: + case 46128: + case 135: + case 148: + case 127: + break; + default: + } + while ((d += 2) < 62) + ; + long k = l(e); + return k; + } + long l(int[] a) { + long m = 0; + for (int i = 0; i < a.length; i++) + m = a[i]; + return m; + } + void f(String[] g) { + int i = 2; + for (; i < 20000; ++i) + j(3, h); + } + public static void main(String[] o) { + try { + LoopRotateBadNodeBudget n = new LoopRotateBadNodeBudget(); + n.f(o); + } catch (Exception ex) { + } + } +}