Skip to content

Commit

Permalink
8231565: More node budget asserts in fuzzed tests
Browse files Browse the repository at this point in the history
Backport-of: 902858e
  • Loading branch information
TheRealMDoerr committed Mar 15, 2022
1 parent 18b5f5b commit f8abb70
Show file tree
Hide file tree
Showing 3 changed files with 100 additions and 19 deletions.
2 changes: 1 addition & 1 deletion src/hotspot/share/opto/loopnode.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1377,7 +1377,7 @@ SHENANDOAHGC_ONLY(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) {
Expand Down
36 changes: 18 additions & 18 deletions src/hotspot/share/opto/loopopts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3185,7 +3185,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;
}

Expand All @@ -3210,8 +3211,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)) {
Expand All @@ -3226,7 +3226,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");

Expand All @@ -3246,25 +3246,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.
Expand All @@ -3280,15 +3276,19 @@ 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++;
}
}
}
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",
Expand Down Expand Up @@ -3336,15 +3336,15 @@ 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();
Node* orig_tail_clone = head_clone->in(2);

// 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++) {
Expand Down Expand Up @@ -3400,7 +3400,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)) {
Expand Down
81 changes: 81 additions & 0 deletions test/hotspot/jtreg/compiler/loopopts/LoopRotateBadNodeBudget.java
Original file line number Diff line number Diff line change
@@ -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) {
}
}
}

1 comment on commit f8abb70

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.