From 736fcd49f7cd3aa6f226b2e088415eaf05f97ee8 Mon Sep 17 00:00:00 2001 From: Emanuel Peter Date: Wed, 14 Dec 2022 17:25:49 +0000 Subject: [PATCH] 8296318: use-def assert: special case undetected loops nested in infinite loops Reviewed-by: chagedorn, kvn --- src/hotspot/share/opto/block.cpp | 8 ++- src/hotspot/share/opto/cfgnode.cpp | 41 ++++++++++++ src/hotspot/share/opto/cfgnode.hpp | 4 ++ src/hotspot/share/opto/loopnode.cpp | 25 +------- .../TestUndetectedLoopInInfiniteLoop.java | 62 +++++++++++++++++++ 5 files changed, 115 insertions(+), 25 deletions(-) create mode 100644 test/hotspot/jtreg/compiler/loopopts/TestUndetectedLoopInInfiniteLoop.java diff --git a/src/hotspot/share/opto/block.cpp b/src/hotspot/share/opto/block.cpp index 57bdbd165a4..f729f93b03b 100644 --- a/src/hotspot/share/opto/block.cpp +++ b/src/hotspot/share/opto/block.cpp @@ -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)"); } } } diff --git a/src/hotspot/share/opto/cfgnode.cpp b/src/hotspot/share/opto/cfgnode.cpp index 8afc4d5d844..b829523ca9b 100644 --- a/src/hotspot/share/opto/cfgnode.cpp +++ b/src/hotspot/share/opto/cfgnode.cpp @@ -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: // diff --git a/src/hotspot/share/opto/cfgnode.hpp b/src/hotspot/share/opto/cfgnode.hpp index a41a744d537..e9d4219cf8b 100644 --- a/src/hotspot/share/opto/cfgnode.hpp +++ b/src/hotspot/share/opto/cfgnode.hpp @@ -91,6 +91,10 @@ public: 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; } diff --git a/src/hotspot/share/opto/loopnode.cpp b/src/hotspot/share/opto/loopnode.cpp index 17112264706..a06773435b6 100644 --- a/src/hotspot/share/opto/loopnode.cpp +++ b/src/hotspot/share/opto/loopnode.cpp @@ -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 diff --git a/test/hotspot/jtreg/compiler/loopopts/TestUndetectedLoopInInfiniteLoop.java b/test/hotspot/jtreg/compiler/loopopts/TestUndetectedLoopInInfiniteLoop.java new file mode 100644 index 00000000000..868a9f27d56 --- /dev/null +++ b/test/hotspot/jtreg/compiler/loopopts/TestUndetectedLoopInInfiniteLoop.java @@ -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); + } + } + } +}