Skip to content

Commit

Permalink
8296318: use-def assert: special case undetected loops nested in infi…
Browse files Browse the repository at this point in the history
…nite loops

Reviewed-by: chagedorn, kvn
  • Loading branch information
eme64 committed Dec 14, 2022
1 parent c05dbac commit 736fcd4
Show file tree
Hide file tree
Showing 5 changed files with 115 additions and 25 deletions.
8 changes: 7 additions & 1 deletion src/hotspot/share/opto/block.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1375,7 +1375,13 @@ void PhaseCFG::verify() const {
}
}
}
assert(is_loop || block->find_node(def) < j, "uses must follow definitions");
// Uses must be before definition, except if:
// - We are in some kind of loop we already detected
// - We are in infinite loop, where Region may not have been turned into LoopNode
assert(block->find_node(def) < j ||
is_loop ||
(n->is_Phi() && block->head()->as_Region()->is_in_infinite_subgraph()),
"uses must follow definitions (except in loops)");
}
}
}
Expand Down
41 changes: 41 additions & 0 deletions src/hotspot/share/opto/cfgnode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,47 @@ bool RegionNode::is_unreachable_from_root(const PhaseGVN* phase) const {
return true; // The Region node is unreachable - it is dead.
}

#ifdef ASSERT
// Is this region in an infinite subgraph?
// (no path to root except through false NeverBranch exit)
bool RegionNode::is_in_infinite_subgraph() {
ResourceMark rm;
Unique_Node_List worklist;
worklist.push(this);
return RegionNode::are_all_nodes_in_infinite_subgraph(worklist);
}

// Are all nodes in worklist in infinite subgraph?
// (no path to root except through false NeverBranch exit)
// worklist is directly used for the traversal
bool RegionNode::are_all_nodes_in_infinite_subgraph(Unique_Node_List& worklist) {
// BFS traversal down the CFG, except through NeverBranch exits
for (uint i = 0; i < worklist.size(); ++i) {
Node* n = worklist.at(i);
assert(n->is_CFG(), "only traverse CFG");
if (n->is_Root()) {
// Found root -> there was an exit!
return false;
} else if (n->is_NeverBranch()) {
// Only follow the loop-internal projection, not the NeverBranch exit
ProjNode* proj = n->as_NeverBranch()->proj_out_or_null(0);
assert(proj != nullptr, "must find loop-internal projection of NeverBranch");
worklist.push(proj);
} else {
// Traverse all CFG outputs
for (DUIterator_Fast imax, i = n->fast_outs(imax); i < imax; i++) {
Node* use = n->fast_out(i);
if (use->is_CFG()) {
worklist.push(use);
}
}
}
}
// No exit found for any loop -> all are infinite
return true;
}
#endif //ASSERT

bool RegionNode::try_clean_mem_phi(PhaseGVN *phase) {
// Incremental inlining + PhaseStringOpts sometimes produce:
//
Expand Down
4 changes: 4 additions & 0 deletions src/hotspot/share/opto/cfgnode.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,10 @@ class RegionNode : public Node {
PhiNode* has_unique_phi() const; // returns the unique phi user, or NULL
// Is this region node unreachable from root?
bool is_unreachable_region(const PhaseGVN* phase);
#ifdef ASSERT
bool is_in_infinite_subgraph();
static bool are_all_nodes_in_infinite_subgraph(Unique_Node_List& worklist);
#endif //ASSERT
virtual int Opcode() const;
virtual uint size_of() const { return sizeof(*this); }
virtual bool pinned() const { return (const Node*)in(0) == this; }
Expand Down
25 changes: 1 addition & 24 deletions src/hotspot/share/opto/loopnode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4201,30 +4201,7 @@ bool PhaseIdealLoop::only_has_infinite_loops() {
assert(head->is_Region(), "");
worklist.push(head);
}
// BFS traversal down the CFG, except through NeverBranch exits
for (uint i = 0; i < worklist.size(); ++i) {
Node* n = worklist.at(i);
assert(n->is_CFG(), "only traverse CFG");
if (n->is_Root()) {
// Found root -> there was an exit!
return false;
} else if (n->is_NeverBranch()) {
// Only follow the loop-internal projection, not the NeverBranch exit
ProjNode* proj = n->as_NeverBranch()->proj_out_or_null(0);
assert(proj != nullptr, "must find loop-internal projection of NeverBranch");
worklist.push(proj);
} else {
// Traverse all CFG outputs
for (DUIterator_Fast imax, i = n->fast_outs(imax); i < imax; i++) {
Node* use = n->fast_out(i);
if (use->is_CFG()) {
worklist.push(use);
}
}
}
}
// No exit found for any loop -> all are infinite
return true;
return RegionNode::are_all_nodes_in_infinite_subgraph(worklist);
}
#endif

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/*
* 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 8296318
* @summary Loops inside infinite loops may not be detected, thus a region may still
* be the loop head, even if it is not a LoopNode.
* @run main/othervm -Xcomp -XX:-TieredCompilation
* -XX:CompileCommand=compileonly,TestUndetectedLoopInInfiniteLoop::test*
* -XX:PerMethodTrapLimit=0
* TestUndetectedLoopInInfiniteLoop
*/


public class TestUndetectedLoopInInfiniteLoop {
public static void main (String[] args) {
test(true, false);
}
public static void test(boolean flag, boolean flag2) {
int x = 0;
if (flag2) { // runtime check, avoid infinite loop
while (true) { // infinite loop (no exit)
if (flag) {
x++;
}
do { // inner loop
// assert for this block
// Region
// Phi -> SubI -> XorI ---> Phi
x = (x - 1) ^ 1;
// Problem: this looks like a loop, but we have no LoopNode
// We have no LoopNode, because this is all in an infinite
// loop, and during PhaseIdealLoop::build_loop_tree we do not
// attach the loops of an infinite loop to the loop tree,
// and hence we do not get to call beautify_loop on these loops
// which would have turned the Region into a LoopNode.
} while (x < 0);
}
}
}
}

3 comments on commit 736fcd4

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

@GoeLin
Copy link
Member

@GoeLin GoeLin commented on 736fcd4 Apr 16, 2023

Choose a reason for hiding this comment

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

/backport jdk17u-dev

@openjdk
Copy link

@openjdk openjdk bot commented on 736fcd4 Apr 16, 2023

Choose a reason for hiding this comment

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

@GoeLin the backport was successfully created on the branch GoeLin-backport-736fcd49 in my personal fork of openjdk/jdk17u-dev. To create a pull request with this backport targeting openjdk/jdk17u-dev:master, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit 736fcd49 from the openjdk/jdk repository.

The commit being backported was authored by Emanuel Peter on 14 Dec 2022 and was reviewed by Christian Hagedorn and Vladimir Kozlov.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jdk17u-dev:

$ git fetch https://github.com/openjdk-bots/jdk17u-dev.git GoeLin-backport-736fcd49:GoeLin-backport-736fcd49
$ git checkout GoeLin-backport-736fcd49
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jdk17u-dev.git GoeLin-backport-736fcd49

Please sign in to comment.