From 4c1540baa61e65fc20451a2d9de09b23fb7baf64 Mon Sep 17 00:00:00 2001 From: Tobias Holenstein Date: Wed, 15 Nov 2023 08:31:39 +0000 Subject: [PATCH] 8287284: C2: loop optimization performs split_thru_phi infinitely many times Reviewed-by: thartmann, epeter, chagedorn, roland --- src/hotspot/share/opto/loopnode.hpp | 2 + src/hotspot/share/opto/loopopts.cpp | 27 ++++++++- .../loopopts/TestSplitThruPhiInfinitely.java | 57 +++++++++++++++++++ 3 files changed, 84 insertions(+), 2 deletions(-) create mode 100644 test/hotspot/jtreg/compiler/loopopts/TestSplitThruPhiInfinitely.java diff --git a/src/hotspot/share/opto/loopnode.hpp b/src/hotspot/share/opto/loopnode.hpp index 1804f1c7702..4a34d7e49ba 100644 --- a/src/hotspot/share/opto/loopnode.hpp +++ b/src/hotspot/share/opto/loopnode.hpp @@ -1735,6 +1735,8 @@ public: Node* similar_subtype_check(const Node* x, Node* r_in); void update_addp_chain_base(Node* x, Node* old_base, Node* new_base); + + bool can_move_to_inner_loop(Node* n, LoopNode* n_loop, Node* x); }; diff --git a/src/hotspot/share/opto/loopopts.cpp b/src/hotspot/share/opto/loopopts.cpp index 0a197dd413b..ea1bdc933d0 100644 --- a/src/hotspot/share/opto/loopopts.cpp +++ b/src/hotspot/share/opto/loopopts.cpp @@ -152,9 +152,22 @@ Node* PhaseIdealLoop::split_thru_phi(Node* n, Node* region, int policy) { } } } - if (x != the_clone && the_clone != nullptr) - _igvn.remove_dead_node(the_clone); + phi->set_req( i, x ); + + if (the_clone == nullptr) { + continue; + } + + if (the_clone != x) { + _igvn.remove_dead_node(the_clone); + } else if (region->is_Loop() && i == LoopNode::LoopBackControl && + n->is_Load() && can_move_to_inner_loop(n, region->as_Loop(), x)) { + // it is not a win if 'x' moved from an outer to an inner loop + // this edge case can only happen for Load nodes + wins = 0; + break; + } } // Too few wins? if (wins <= policy) { @@ -218,6 +231,16 @@ Node* PhaseIdealLoop::split_thru_phi(Node* n, Node* region, int policy) { return phi; } +// Test whether node 'x' can move into an inner loop relative to node 'n'. +// Note: The test is not exact. Returns true if 'x' COULD end up in an inner loop, +// BUT it can also return true and 'x' is in the outer loop +bool PhaseIdealLoop::can_move_to_inner_loop(Node* n, LoopNode* n_loop, Node* x) { + IdealLoopTree* n_loop_tree = get_loop(n_loop); + IdealLoopTree* x_loop_tree = get_loop(get_early_ctrl(x)); + // x_loop_tree should be outer or same loop as n_loop_tree + return !x_loop_tree->is_member(n_loop_tree); +} + // Subtype checks that carry profile data don't common so look for a replacement by following edges Node* PhaseIdealLoop::similar_subtype_check(const Node* x, Node* r_in) { if (x->is_SubTypeCheck()) { diff --git a/test/hotspot/jtreg/compiler/loopopts/TestSplitThruPhiInfinitely.java b/test/hotspot/jtreg/compiler/loopopts/TestSplitThruPhiInfinitely.java new file mode 100644 index 00000000000..07055970e62 --- /dev/null +++ b/test/hotspot/jtreg/compiler/loopopts/TestSplitThruPhiInfinitely.java @@ -0,0 +1,57 @@ +/* + * Copyright (c) 2023, 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 8287284 + * @summary The phi of cnt is split from the inner to the outer loop, + * and then from outer loop to the inner loop again. + * This ended in a endless optimization cycle. + * @library /test/lib / + * @run driver compiler.c2.loopopts.TestSplitThruPhiInfinitely + */ + +package compiler.c2.loopopts; + +import compiler.lib.ir_framework.*; + +public class TestSplitThruPhiInfinitely { + + public static int cnt = 1; + + @Test + @IR(counts = {IRNode.PHI, " <= 10"}) + public static void test() { + int j = 0; + do { + j = cnt; + for (int k = 0; k < 20000; k++) { + cnt += 2; + } + } while (++j < 10); + } + + public static void main(String[] args) { + TestFramework.runWithFlags("-XX:-PartialPeelLoop"); + } +}