From a109b64724ff5e1245c8806706aa556d98006746 Mon Sep 17 00:00:00 2001 From: Tobias Hartmann Date: Mon, 26 Feb 2024 09:27:12 +0100 Subject: [PATCH 1/5] 8326638: Crash in PhaseIdealLoop::remix_address_expressions due to unexpected Region instead of Loop --- src/hotspot/share/opto/loopopts.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/hotspot/share/opto/loopopts.cpp b/src/hotspot/share/opto/loopopts.cpp index 0010a7cd90353..1f20ee9d9ba98 100644 --- a/src/hotspot/share/opto/loopopts.cpp +++ b/src/hotspot/share/opto/loopopts.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 1999, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1999, 2024, 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 @@ -583,7 +583,9 @@ Node* PhaseIdealLoop::remix_address_expressions(Node* n) { n23_loop == n_loop) { Node* add1 = new AddPNode(n->in(1), n->in(2)->in(2), n->in(3)); // Stuff new AddP in the loop preheader - register_new_node(add1, n_loop->_head->as_Loop()->skip_strip_mined(1)->in(LoopNode::EntryControl)); + Node* entry = n_loop->_head->is_Loop() ? n_loop->_head->as_Loop()->skip_strip_mined(1)->in(LoopNode::EntryControl) + : n_loop->_head->in(LoopNode::EntryControl); + register_new_node(add1, entry); Node* add2 = new AddPNode(n->in(1), add1, n->in(2)->in(3)); register_new_node(add2, n_ctrl); _igvn.replace_node(n, add2); @@ -604,7 +606,9 @@ Node* PhaseIdealLoop::remix_address_expressions(Node* n) { if (!is_member(n_loop,get_ctrl(I))) { Node* add1 = new AddPNode(n->in(1), n->in(2), I); // Stuff new AddP in the loop preheader - register_new_node(add1, n_loop->_head->as_Loop()->skip_strip_mined(1)->in(LoopNode::EntryControl)); + Node* entry = n_loop->_head->is_Loop() ? n_loop->_head->as_Loop()->skip_strip_mined(1)->in(LoopNode::EntryControl) + : n_loop->_head->in(LoopNode::EntryControl); + register_new_node(add1, entry); Node* add2 = new AddPNode(n->in(1), add1, V); register_new_node(add2, n_ctrl); _igvn.replace_node(n, add2); From 48c6427da7e688382a8c9278267d131b442082de Mon Sep 17 00:00:00 2001 From: Tobias Hartmann Date: Mon, 26 Feb 2024 09:31:59 +0100 Subject: [PATCH 2/5] Added test --- ...AddressExpressionsWithIrreducibleLoop.java | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 test/hotspot/jtreg/compiler/loopopts/TestRemixAddressExpressionsWithIrreducibleLoop.java diff --git a/test/hotspot/jtreg/compiler/loopopts/TestRemixAddressExpressionsWithIrreducibleLoop.java b/test/hotspot/jtreg/compiler/loopopts/TestRemixAddressExpressionsWithIrreducibleLoop.java new file mode 100644 index 0000000000000..50b2e3cade8c9 --- /dev/null +++ b/test/hotspot/jtreg/compiler/loopopts/TestRemixAddressExpressionsWithIrreducibleLoop.java @@ -0,0 +1,57 @@ +/* + * Copyright (c) 2024, 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 8326638 + * @summary TODO + * @run main/othervm -XX:-TieredCompilation -Xbatch + * -XX:CompileCommand=compileonly,TestRemixAddressExpressionsWithIrreducibleLoop::test + * TestRemixAddressExpressionsWithIrreducibleLoop + */ + +public class TestRemixAddressExpressionsWithIrreducibleLoop { + + public static void main(String[] args) { + test("4"); + } + + public static void test(String arg) { + for (int i = 0; i < 100_000; ++i) { + int j = 0; + while (true) { + boolean tmp = "1\ufff0".startsWith(arg, 2 - arg.length()); + if (j++ > 100) + break; + } + loop: + while (i >= 100) { + for (int i2 = 0; i2 < 1; i2 = 1) + if (j > 300) + break loop; + j++; + } + } + } +} From 70001cfbfed28e2b3278dbdf4b817ca625277284 Mon Sep 17 00:00:00 2001 From: Tobias Hartmann Date: Mon, 26 Feb 2024 09:33:07 +0100 Subject: [PATCH 3/5] Missing comment --- .../TestRemixAddressExpressionsWithIrreducibleLoop.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/hotspot/jtreg/compiler/loopopts/TestRemixAddressExpressionsWithIrreducibleLoop.java b/test/hotspot/jtreg/compiler/loopopts/TestRemixAddressExpressionsWithIrreducibleLoop.java index 50b2e3cade8c9..ba7be18d89a5b 100644 --- a/test/hotspot/jtreg/compiler/loopopts/TestRemixAddressExpressionsWithIrreducibleLoop.java +++ b/test/hotspot/jtreg/compiler/loopopts/TestRemixAddressExpressionsWithIrreducibleLoop.java @@ -25,7 +25,7 @@ /* * @test * @bug 8326638 - * @summary TODO + * @summary Test handling of irreducible loops in PhaseIdealLoop::remix_address_expressions. * @run main/othervm -XX:-TieredCompilation -Xbatch * -XX:CompileCommand=compileonly,TestRemixAddressExpressionsWithIrreducibleLoop::test * TestRemixAddressExpressionsWithIrreducibleLoop From a6559ceeb27e7bb279e1d057e5160e2d2ddf1504 Mon Sep 17 00:00:00 2001 From: Tobias Hartmann Date: Mon, 26 Feb 2024 09:36:25 +0100 Subject: [PATCH 4/5] Removed whitespace --- .../TestRemixAddressExpressionsWithIrreducibleLoop.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/hotspot/jtreg/compiler/loopopts/TestRemixAddressExpressionsWithIrreducibleLoop.java b/test/hotspot/jtreg/compiler/loopopts/TestRemixAddressExpressionsWithIrreducibleLoop.java index ba7be18d89a5b..ff7be96007de7 100644 --- a/test/hotspot/jtreg/compiler/loopopts/TestRemixAddressExpressionsWithIrreducibleLoop.java +++ b/test/hotspot/jtreg/compiler/loopopts/TestRemixAddressExpressionsWithIrreducibleLoop.java @@ -26,7 +26,7 @@ * @test * @bug 8326638 * @summary Test handling of irreducible loops in PhaseIdealLoop::remix_address_expressions. - * @run main/othervm -XX:-TieredCompilation -Xbatch + * @run main/othervm -XX:-TieredCompilation -Xbatch * -XX:CompileCommand=compileonly,TestRemixAddressExpressionsWithIrreducibleLoop::test * TestRemixAddressExpressionsWithIrreducibleLoop */ From 12d59c1d91f5eee5f7a289faa5cbd266f847ef49 Mon Sep 17 00:00:00 2001 From: Tobias Hartmann Date: Tue, 27 Feb 2024 09:54:25 +0100 Subject: [PATCH 5/5] Skip optimization for irreducible loops --- src/hotspot/share/opto/loopopts.cpp | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/hotspot/share/opto/loopopts.cpp b/src/hotspot/share/opto/loopopts.cpp index 1f20ee9d9ba98..496cac3d8fe33 100644 --- a/src/hotspot/share/opto/loopopts.cpp +++ b/src/hotspot/share/opto/loopopts.cpp @@ -571,8 +571,8 @@ Node* PhaseIdealLoop::remix_address_expressions(Node* n) { } // Replace ((I1 +p V) +p I2) with ((I1 +p I2) +p V), - // but not if I2 is a constant. - if (n_op == Op_AddP) { + // but not if I2 is a constant. Skip for irreducible loops. + if (n_op == Op_AddP && n_loop->_head->is_Loop()) { if (n2_loop == n_loop && n3_loop != n_loop) { if (n->in(2)->Opcode() == Op_AddP && !n->in(3)->is_Con()) { Node* n22_ctrl = get_ctrl(n->in(2)->in(2)); @@ -583,9 +583,7 @@ Node* PhaseIdealLoop::remix_address_expressions(Node* n) { n23_loop == n_loop) { Node* add1 = new AddPNode(n->in(1), n->in(2)->in(2), n->in(3)); // Stuff new AddP in the loop preheader - Node* entry = n_loop->_head->is_Loop() ? n_loop->_head->as_Loop()->skip_strip_mined(1)->in(LoopNode::EntryControl) - : n_loop->_head->in(LoopNode::EntryControl); - register_new_node(add1, entry); + register_new_node(add1, n_loop->_head->as_Loop()->skip_strip_mined(1)->in(LoopNode::EntryControl)); Node* add2 = new AddPNode(n->in(1), add1, n->in(2)->in(3)); register_new_node(add2, n_ctrl); _igvn.replace_node(n, add2); @@ -606,9 +604,7 @@ Node* PhaseIdealLoop::remix_address_expressions(Node* n) { if (!is_member(n_loop,get_ctrl(I))) { Node* add1 = new AddPNode(n->in(1), n->in(2), I); // Stuff new AddP in the loop preheader - Node* entry = n_loop->_head->is_Loop() ? n_loop->_head->as_Loop()->skip_strip_mined(1)->in(LoopNode::EntryControl) - : n_loop->_head->in(LoopNode::EntryControl); - register_new_node(add1, entry); + register_new_node(add1, n_loop->_head->as_Loop()->skip_strip_mined(1)->in(LoopNode::EntryControl)); Node* add2 = new AddPNode(n->in(1), add1, V); register_new_node(add2, n_ctrl); _igvn.replace_node(n, add2);