stan/jdk-24
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

8284358: Unreachable loop is not removed from C2 IR, leading to a broken graph

Browse Source 
Co-authored-by: Christian Hagedorn <chagedorn@openjdk.org>
Reviewed-by: kvn, chagedorn
This commit is contained in:
Tobias Hartmann 2022-07-01 05:23:57 +00:00
parent c20b3aa9c5
commit 95497772e7
2 changed files with 286 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

86
src/hotspot/share/opto/cfgnode.cpp
View File

@ -314,7 +314,7 @@ static bool check_compare_clipping( bool less_than, IfNode *iff, ConNode *limit,
}
//------------------------------is_unreachable_region--------------------------
// Find if the Region node is reachable from the root.
// Check if the RegionNode is part of an unsafe loop and unreachable from root.
bool RegionNode::is_unreachable_region(const PhaseGVN* phase) {
Node* top = phase->C->top();
assert(req() == 2 || (req() == 3 && in(1) != NULL && in(2) == top), "sanity check arguments");
@ -373,7 +373,7 @@ bool RegionNode::is_unreachable_from_root(const PhaseGVN* phase) const {
VectorSet visited;
// Mark all control nodes reachable from root outputs
Node *n = (Node*)phase->C->root();
Node* n = (Node*)phase->C->root();
nstack.push(n);
visited.set(n->_idx);
while (nstack.size() != 0) {
@ -475,7 +475,7 @@ Node *RegionNode::Ideal(PhaseGVN *phase, bool can_reshape) {
// Remove TOP or NULL input paths. If only 1 input path remains, this Region
// degrades to a copy.
bool add_to_worklist = false;
bool add_to_worklist = true;
bool modified = false;
int cnt = 0; // Count of values merging
DEBUG_ONLY( int cnt_orig = req(); ) // Save original inputs count
@ -501,7 +501,7 @@ Node *RegionNode::Ideal(PhaseGVN *phase, bool can_reshape) {
}
}
if( phase->type(n) == Type::TOP ) {
set_req(i, NULL); // Ignore TOP inputs
set_req_X(i, NULL, phase); // Ignore TOP inputs
modified = true;
i--;
continue;
@ -532,7 +532,8 @@ Node *RegionNode::Ideal(PhaseGVN *phase, bool can_reshape) {
}
}
}
add_to_worklist = true;
add_to_worklist = false;
phase->is_IterGVN()->add_users_to_worklist(this);
i--;
}
}
@ -547,45 +548,51 @@ Node *RegionNode::Ideal(PhaseGVN *phase, bool can_reshape) {
if ((this->is_Loop() && (del_it == LoopNode::EntryControl ||
(del_it == 0 && is_unreachable_region(phase)))) ||
(!this->is_Loop() && has_phis && is_unreachable_region(phase))) {
// Yes, the region will be removed during the next step below.
// Cut the backedge input and remove phis since no data paths left.
// We don't cut outputs to other nodes here since we need to put them
// on the worklist.
PhaseIterGVN *igvn = phase->is_IterGVN();
if (in(1)->outcnt() == 1) {
igvn->_worklist.push(in(1));
}
del_req(1);
cnt = 0;
assert( req() == 1, "no more inputs expected" );
uint max = outcnt();
bool progress = true;
Node *top = phase->C->top();
DUIterator j;
while(progress) {
progress = false;
for (j = outs(); has_out(j); j++) {
Node *n = out(j);
if( n->is_Phi() ) {
assert(n->in(0) == this, "");
assert( n->req() == 2 && n->in(1) != NULL, "Only one data input expected" );
// Break dead loop data path.
// Eagerly replace phis with top to avoid regionless phis.
igvn->replace_node(n, top);
if( max != outcnt() ) {
progress = true;
j = refresh_out_pos(j);
max = outcnt();
// This region and therefore all nodes on the input control path(s) are unreachable
// from root. To avoid incomplete removal of unreachable subgraphs, walk up the CFG
// and aggressively replace all nodes by top.
PhaseIterGVN* igvn = phase->is_IterGVN();
Node* top = phase->C->top();
ResourceMark rm;
Node_List nstack;
VectorSet visited;
nstack.push(this);
visited.set(_idx);
while (nstack.size() != 0) {
Node* n = nstack.pop();
for (uint i = 0; i < n->req(); ++i) {
Node* m = n->in(i);
assert(m != (Node*)phase->C->root(), "Should be unreachable from root");
if (m != NULL && m->is_CFG() && !visited.test_set(m->_idx)) {
nstack.push(m);
}
}
if (n->is_Region()) {
// Eagerly replace phis with top to avoid regionless phis.
n->set_req(0, NULL);
bool progress = true;
uint max = n->outcnt();
DUIterator j;
while (progress) {
progress = false;
for (j = n->outs(); n->has_out(j); j++) {
Node* u = n->out(j);
if (u->is_Phi()) {
igvn->replace_node(u, top);
if (max != n->outcnt()) {
progress = true;
j = n->refresh_out_pos(j);
max = n->outcnt();
}
}
}
}
}
igvn->replace_node(n, top);
}
add_to_worklist = true;
return NULL;
}
}
if (add_to_worklist) {
phase->is_IterGVN()->add_users_to_worklist(this); // Revisit collapsed Phis
}
if( cnt <= 1 ) { // Only 1 path in?
set_req(0, NULL); // Null control input for region copy
@ -629,8 +636,9 @@ Node *RegionNode::Ideal(PhaseGVN *phase, bool can_reshape) {
assert(parent_ctrl != NULL, "Region is a copy of some non-null control");
assert(parent_ctrl != this, "Close dead loop");
}
if (!add_to_worklist)
if (add_to_worklist) {
igvn->add_users_to_worklist(this); // Check for further allowed opts
}
for (DUIterator_Last imin, i = last_outs(imin); i >= imin; --i) {
Node* n = last_out(i);
igvn->hash_delete(n); // Remove from worklist before modifying edges

239
test/hotspot/jtreg/compiler/c2/TestDeadDataLoop.java Normal file
View File

@ -0,0 +1,239 @@
/*
* Copyright (c) 2022, 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 8284358
* @summary An unreachable loop is not removed, leading to a broken graph.
* @requires vm.compiler2.enabled
* @run main/othervm -Xcomp -XX:-TieredCompilation -XX:+UnlockDiagnosticVMOptions -XX:+StressIGVN -XX:StressSeed=1448005075
* -XX:CompileCommand=compileonly,*TestDeadDataLoop::test* -XX:CompileCommand=dontinline,*TestDeadDataLoop::notInlined
* compiler.c2.TestDeadDataLoop
* @run main/othervm -Xcomp -XX:-TieredCompilation -XX:+UnlockDiagnosticVMOptions -XX:+StressIGVN -XX:StressSeed=1922737097
* -XX:CompileCommand=compileonly,*TestDeadDataLoop::test* -XX:CompileCommand=dontinline,*TestDeadDataLoop::notInlined
* compiler.c2.TestDeadDataLoop
* @run main/othervm -Xcomp -XX:-TieredCompilation -XX:+UnlockDiagnosticVMOptions -XX:+StressIGVN
* -XX:CompileCommand=compileonly,*TestDeadDataLoop::test* -XX:CompileCommand=dontinline,*TestDeadDataLoop::notInlined
* compiler.c2.TestDeadDataLoop
*/
package compiler.c2;
public class TestDeadDataLoop {
static class MyValue {
final int x;
MyValue(int x) {
this.x = x;
}
}
static boolean flag = false;
static MyValue escape = null;
static volatile int volInt = 0;
static boolean test1() {
Integer box;
if (flag) {
box = 0;
} else {
box = 1;
}
if (box == 2) {
// Not reachable but that's only known after Incremental Boxing Inline
for (int i = 0; i < 1000;) {
if (notInlined()) {
break;
}
}
MyValue val = new MyValue(4);
escape = new MyValue(42);
// Trigger scalarization of val in safepoint debug info
notInlined();
if (val.x < 0) {
return true;
}
}
return false;
}
static boolean test2() {
MyValue box = new MyValue(1);
if (box.x == 0) {
// Not reachable but that's only known once the box.x load is folded during IGVN
for (int i = 0; i < 1000;) {
if (notInlined()) {
break;
}
}
MyValue val = new MyValue(4);
escape = new MyValue(42);
// Trigger scalarization of val in safepoint debug info
notInlined();
if (val.x < 0) {
return true;
}
}
return false;
}
static void test3() {
Integer box = 0;
if (flag) {
box = 1;
}
if (box == 2) {
for (int i = 0; i < 1000;) {
if (notInlined()) {
break;
}
}
escape = new MyValue(42);
}
}
static void test4() {
MyValue box = new MyValue(1);
if (box.x == 0) {
for (int i = 0; i < 1000;) {
if (notInlined()) {
break;
}
}
escape = new MyValue(42);
}
}
static void test5() {
MyValue box = new MyValue(1);
if (box.x == 0) {
while (true) {
if (notInlined()) {
break;
}
}
escape = new MyValue(42);
}
}
static void test6() {
MyValue box = new MyValue(1);
if (box.x == 0) {
while (notInlined()) { }
escape = new MyValue(42);
}
}
static void test7() {
MyValue box = new MyValue(1);
if (box.x == 0) {
while (true) {
escape = new MyValue(2);
if (notInlined()) {
break;
}
}
escape = new MyValue(42);
}
}
static void test8() {
MyValue box = new MyValue(1);
if (box.x == 0) {
for (int i = 0; i < 1000;) {
notInlined();
if (flag) {
break;
}
}
escape = new MyValue(42);
}
}
static boolean test9() {
Integer box;
if (flag) {
box = 0;
} else {
box = 1;
}
if (box == 2) {
for (int i = 0; i < 1000;) {
// MemBarRelease as only Phi user
volInt = 42;
if (flag) {
break;
}
}
MyValue val = new MyValue(4);
escape = new MyValue(42);
notInlined();
if (val.x < 0) {
return true;
}
}
return false;
}
static void test10() {
MyValue box = new MyValue(1);
if (box.x == 0) {
while (true) {
// Allocate node as only Phi user
escape = new MyValue(2);
if (flag) {
break;
}
}
escape = new MyValue(42);
}
}
public static boolean notInlined() {
return false;
}
public static void main(String[] args) {
// Make sure classes are initialized
Integer i = 42;
new MyValue(i);
test1();
test2();
test3();
test4();
test5();
test6();
test7();
test8();
test9();
test10();
}
}