From d5af551e83590dd04d0b623aa78a754912c1041e Mon Sep 17 00:00:00 2001 From: rwestrel Date: Fri, 8 Sep 2023 15:46:01 +0200 Subject: [PATCH 1/7] fix & test --- src/hotspot/share/opto/loopTransform.cpp | 2 + src/hotspot/share/opto/loopnode.hpp | 2 +- src/hotspot/share/opto/loopopts.cpp | 29 +++------ .../TestBadControlAfterPreMainPost.java | 61 +++++++++++++++++++ 4 files changed, 73 insertions(+), 21 deletions(-) create mode 100644 test/hotspot/jtreg/compiler/loopopts/TestBadControlAfterPreMainPost.java diff --git a/src/hotspot/share/opto/loopTransform.cpp b/src/hotspot/share/opto/loopTransform.cpp index 51647802efaa8..9a78ebf661751 100644 --- a/src/hotspot/share/opto/loopTransform.cpp +++ b/src/hotspot/share/opto/loopTransform.cpp @@ -1918,6 +1918,8 @@ Node *PhaseIdealLoop::insert_post_loop(IdealLoopTree* loop, Node_List& old_new, post_head->set_normal_loop(); post_head->set_post_loop(main_head); + main_exit = outer_main_end->proj_out(false); + // Reduce the post-loop trip count. CountedLoopEndNode* post_end = old_new[main_end->_idx]->as_CountedLoopEnd(); post_end->_prob = PROB_FAIR; diff --git a/src/hotspot/share/opto/loopnode.hpp b/src/hotspot/share/opto/loopnode.hpp index 7544ef98da7aa..5ccb172b5bd9f 100644 --- a/src/hotspot/share/opto/loopnode.hpp +++ b/src/hotspot/share/opto/loopnode.hpp @@ -1543,7 +1543,7 @@ class PhaseIdealLoop : public PhaseTransform { Node *find_use_block( Node *use, Node *def, Node *old_false, Node *new_false, Node *old_true, Node *new_true ); void handle_use( Node *use, Node *def, small_cache *cache, Node *region_dom, Node *new_false, Node *new_true, Node *old_false, Node *old_true ); bool split_up( Node *n, Node *blk1, Node *blk2 ); - void sink_use( Node *use, Node *post_loop ); + Node* place_outside_loop(Node* useblock, IdealLoopTree* loop) const; Node* try_move_store_before_loop(Node* n, Node *n_ctrl); void try_move_store_after_loop(Node* n); diff --git a/src/hotspot/share/opto/loopopts.cpp b/src/hotspot/share/opto/loopopts.cpp index 8ccb0a8cdb908..3b05a176248a9 100644 --- a/src/hotspot/share/opto/loopopts.cpp +++ b/src/hotspot/share/opto/loopopts.cpp @@ -2064,17 +2064,6 @@ CmpNode*PhaseIdealLoop::clone_bool(PhiNode* phi) { return (CmpNode*)cmp; } -//------------------------------sink_use--------------------------------------- -// If 'use' was in the loop-exit block, it now needs to be sunk -// below the post-loop merge point. -void PhaseIdealLoop::sink_use( Node *use, Node *post_loop ) { - if (!use->is_CFG() && get_ctrl(use) == post_loop->in(2)) { - set_ctrl(use, post_loop); - for (DUIterator j = use->outs(); use->has_out(j); j++) - sink_use(use->out(j), post_loop); - } -} - void PhaseIdealLoop::clone_loop_handle_data_uses(Node* old, Node_List &old_new, IdealLoopTree* loop, IdealLoopTree* outer_loop, Node_List*& split_if_set, Node_List*& split_bool_set, @@ -2141,7 +2130,7 @@ void PhaseIdealLoop::clone_loop_handle_data_uses(Node* old, Node_List &old_new, while( use->in(idx) != old ) idx++; Node *prev = use->is_CFG() ? use : get_ctrl(use); assert(!loop->is_member(get_loop(prev)) && !outer_loop->is_member(get_loop(prev)), "" ); - Node *cfg = prev->_idx >= new_counter + Node *cfg = (prev->_idx >= new_counter && prev->is_Region()) ? prev->in(2) : idom(prev); if( use->is_Phi() ) // Phi use is in prior block @@ -2165,7 +2154,7 @@ void PhaseIdealLoop::clone_loop_handle_data_uses(Node* old, Node_List &old_new, while(!outer_loop->is_member(get_loop(cfg))) { prev = cfg; - cfg = cfg->_idx >= new_counter ? cfg->in(2) : idom(cfg); + cfg = (cfg->_idx >= new_counter && cfg->is_Region()) ? cfg->in(2) : idom(cfg); } // If the use occurs after merging several exits from the loop, then // old value must have dominated all those exits. Since the same old @@ -2223,10 +2212,6 @@ void PhaseIdealLoop::clone_loop_handle_data_uses(Node* old, Node_List &old_new, if( hit ) // Go ahead and re-hash for hits. _igvn.replace_node( use, hit ); } - - // If 'use' was in the loop-exit block, it now needs to be sunk - // below the post-loop merge point. - sink_use( use, prev ); } } } @@ -2593,8 +2578,6 @@ void PhaseIdealLoop::fix_ctrl_uses(const Node_List& body, const IdealLoopTree* l // We need a Region to merge the exit from the peeled body and the // exit from the old loop body. RegionNode *r = new RegionNode(3); - // Map the old use to the new merge point - old_new.map( use->_idx, r ); uint dd_r = MIN2(dom_depth(newuse), dom_depth(use)); assert(dd_r >= dom_depth(dom_lca(newuse, use)), "" ); @@ -2630,12 +2613,18 @@ void PhaseIdealLoop::fix_ctrl_uses(const Node_List& body, const IdealLoopTree* l l -= uses_found; // we deleted 1 or more copies of this edge } + assert(use->is_Proj(), "loop exit should be projections"); + Node* use_clone = use->clone(); + register_control(use_clone, use_loop, idom(use), dom_depth(use)); // Now finish up 'r' r->set_req(1, newuse); - r->set_req(2, use); + r->set_req(2, use_clone); _igvn.register_new_node_with_optimizer(r); set_loop(r, use_loop); set_idom(r, (side_by_side_idom == nullptr) ? newuse->in(0) : side_by_side_idom, dd_r); + lazy_replace(use, r); + // Map the (cloned) old use to the new merge point + old_new.map( use_clone->_idx, r ); } // End of if a loop-exit test } } diff --git a/test/hotspot/jtreg/compiler/loopopts/TestBadControlAfterPreMainPost.java b/test/hotspot/jtreg/compiler/loopopts/TestBadControlAfterPreMainPost.java new file mode 100644 index 0000000000000..e3d0b564e83fc --- /dev/null +++ b/test/hotspot/jtreg/compiler/loopopts/TestBadControlAfterPreMainPost.java @@ -0,0 +1,61 @@ +/* + * Copyright (c) 2023, Red Hat, Inc. 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 8315920 + * @summary C2: "control input must dominate current control" assert failure + * @requires vm.compiler2.enabled + * @run main/othervm -XX:-BackgroundCompilation -XX:-UseLoopPredicate -XX:-DoEscapeAnalysis TestBadControlAfterPreMainPost + */ + +public class TestBadControlAfterPreMainPost { + private static volatile int volatileField; + + public static void main(String[] args) { + int[] array2 = new int[100]; + for (int i = 0; i < 20_000; i++) { + test(1, array2); + } + } + + private static int test(int j, int[] array2) { + int[] array = new int[10]; + array[j] = 42; + float f = 1; + for (int i = 0; i < 100; i++) { + for (int k = 0; k < 10; k++) { + } + f = f * 2; + } + int v = array[0]; + int i = 0; + do { + synchronized (new Object()) { + } + array2[i + v] = 42; + i++; + } while (i < 100); + return (int)f; + } +} From e1e8cdf6031fe800c9fa1117b597503ff3bfd9fe Mon Sep 17 00:00:00 2001 From: Roland Westrelin <47032191+rwestrel@users.noreply.github.com> Date: Mon, 25 Sep 2023 14:55:04 +0200 Subject: [PATCH 2/7] Update src/hotspot/share/opto/loopopts.cpp Co-authored-by: Tobias Hartmann --- src/hotspot/share/opto/loopopts.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/share/opto/loopopts.cpp b/src/hotspot/share/opto/loopopts.cpp index 3b05a176248a9..5558a1810def4 100644 --- a/src/hotspot/share/opto/loopopts.cpp +++ b/src/hotspot/share/opto/loopopts.cpp @@ -2618,7 +2618,7 @@ void PhaseIdealLoop::fix_ctrl_uses(const Node_List& body, const IdealLoopTree* l register_control(use_clone, use_loop, idom(use), dom_depth(use)); // Now finish up 'r' r->set_req(1, newuse); - r->set_req(2, use_clone); + r->set_req(2, use_clone); _igvn.register_new_node_with_optimizer(r); set_loop(r, use_loop); set_idom(r, (side_by_side_idom == nullptr) ? newuse->in(0) : side_by_side_idom, dd_r); From f49d81f08130ca47459c22ca035ae6494fff99a6 Mon Sep 17 00:00:00 2001 From: Roland Westrelin <47032191+rwestrel@users.noreply.github.com> Date: Mon, 25 Sep 2023 14:55:23 +0200 Subject: [PATCH 3/7] Update src/hotspot/share/opto/loopopts.cpp Co-authored-by: Tobias Hartmann --- src/hotspot/share/opto/loopopts.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/share/opto/loopopts.cpp b/src/hotspot/share/opto/loopopts.cpp index 5558a1810def4..13a6c59eb303b 100644 --- a/src/hotspot/share/opto/loopopts.cpp +++ b/src/hotspot/share/opto/loopopts.cpp @@ -2613,7 +2613,7 @@ void PhaseIdealLoop::fix_ctrl_uses(const Node_List& body, const IdealLoopTree* l l -= uses_found; // we deleted 1 or more copies of this edge } - assert(use->is_Proj(), "loop exit should be projections"); + assert(use->is_Proj(), "loop exit should be projection"); Node* use_clone = use->clone(); register_control(use_clone, use_loop, idom(use), dom_depth(use)); // Now finish up 'r' From 58ed2179521e8f9f47aa088a1612d70a758cad65 Mon Sep 17 00:00:00 2001 From: Roland Westrelin <47032191+rwestrel@users.noreply.github.com> Date: Mon, 25 Sep 2023 14:55:33 +0200 Subject: [PATCH 4/7] Update src/hotspot/share/opto/loopopts.cpp Co-authored-by: Tobias Hartmann --- src/hotspot/share/opto/loopopts.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/share/opto/loopopts.cpp b/src/hotspot/share/opto/loopopts.cpp index 13a6c59eb303b..82eb481f8642d 100644 --- a/src/hotspot/share/opto/loopopts.cpp +++ b/src/hotspot/share/opto/loopopts.cpp @@ -2624,7 +2624,7 @@ void PhaseIdealLoop::fix_ctrl_uses(const Node_List& body, const IdealLoopTree* l set_idom(r, (side_by_side_idom == nullptr) ? newuse->in(0) : side_by_side_idom, dd_r); lazy_replace(use, r); // Map the (cloned) old use to the new merge point - old_new.map( use_clone->_idx, r ); + old_new.map(use_clone->_idx, r); } // End of if a loop-exit test } } From a08458477d18d95e02807598c22a50e2d863dd27 Mon Sep 17 00:00:00 2001 From: Roland Westrelin <47032191+rwestrel@users.noreply.github.com> Date: Mon, 25 Sep 2023 14:55:59 +0200 Subject: [PATCH 5/7] Update src/hotspot/share/opto/loopopts.cpp Co-authored-by: Christian Hagedorn --- src/hotspot/share/opto/loopopts.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/share/opto/loopopts.cpp b/src/hotspot/share/opto/loopopts.cpp index 82eb481f8642d..385bd87432306 100644 --- a/src/hotspot/share/opto/loopopts.cpp +++ b/src/hotspot/share/opto/loopopts.cpp @@ -2130,7 +2130,7 @@ void PhaseIdealLoop::clone_loop_handle_data_uses(Node* old, Node_List &old_new, while( use->in(idx) != old ) idx++; Node *prev = use->is_CFG() ? use : get_ctrl(use); assert(!loop->is_member(get_loop(prev)) && !outer_loop->is_member(get_loop(prev)), "" ); - Node *cfg = (prev->_idx >= new_counter && prev->is_Region()) + Node* cfg = (prev->_idx >= new_counter && prev->is_Region()) ? prev->in(2) : idom(prev); if( use->is_Phi() ) // Phi use is in prior block From 321a72444e6331936b47c2c16ce9c588f714616d Mon Sep 17 00:00:00 2001 From: rwestrel Date: Mon, 25 Sep 2023 15:37:25 +0200 Subject: [PATCH 6/7] comments --- src/hotspot/share/opto/loopTransform.cpp | 1 + src/hotspot/share/opto/loopopts.cpp | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/src/hotspot/share/opto/loopTransform.cpp b/src/hotspot/share/opto/loopTransform.cpp index 9a78ebf661751..9d1e8dd9fd891 100644 --- a/src/hotspot/share/opto/loopTransform.cpp +++ b/src/hotspot/share/opto/loopTransform.cpp @@ -1918,6 +1918,7 @@ Node *PhaseIdealLoop::insert_post_loop(IdealLoopTree* loop, Node_List& old_new, post_head->set_normal_loop(); post_head->set_post_loop(main_head); + // clone_loop() above changes the exit projection main_exit = outer_main_end->proj_out(false); // Reduce the post-loop trip count. diff --git a/src/hotspot/share/opto/loopopts.cpp b/src/hotspot/share/opto/loopopts.cpp index aa245158aca68..72bd445b86fd7 100644 --- a/src/hotspot/share/opto/loopopts.cpp +++ b/src/hotspot/share/opto/loopopts.cpp @@ -2614,6 +2614,12 @@ void PhaseIdealLoop::fix_ctrl_uses(const Node_List& body, const IdealLoopTree* l } assert(use->is_Proj(), "loop exit should be projection"); + // lazy_replace() below moves all nodes that are: + // - control dependent on the loop exit or + // - have control set to the loop exit + // below the post-loop merge point. lazy_replace() takes a dead control as first input. To make it + // possible to use it, the loop exit projection is cloned and become the new exit projection. The initial one + // becomes dead and is "replaced" by the region. Node* use_clone = use->clone(); register_control(use_clone, use_loop, idom(use), dom_depth(use)); // Now finish up 'r' From 886f964b9b59bcde2232600a480ead5b74df974d Mon Sep 17 00:00:00 2001 From: Roland Westrelin <47032191+rwestrel@users.noreply.github.com> Date: Mon, 25 Sep 2023 17:50:28 +0200 Subject: [PATCH 7/7] Update src/hotspot/share/opto/loopopts.cpp Co-authored-by: Tobias Hartmann --- src/hotspot/share/opto/loopopts.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/share/opto/loopopts.cpp b/src/hotspot/share/opto/loopopts.cpp index 72bd445b86fd7..cee426c86c61d 100644 --- a/src/hotspot/share/opto/loopopts.cpp +++ b/src/hotspot/share/opto/loopopts.cpp @@ -2618,7 +2618,7 @@ void PhaseIdealLoop::fix_ctrl_uses(const Node_List& body, const IdealLoopTree* l // - control dependent on the loop exit or // - have control set to the loop exit // below the post-loop merge point. lazy_replace() takes a dead control as first input. To make it - // possible to use it, the loop exit projection is cloned and become the new exit projection. The initial one + // possible to use it, the loop exit projection is cloned and becomes the new exit projection. The initial one // becomes dead and is "replaced" by the region. Node* use_clone = use->clone(); register_control(use_clone, use_loop, idom(use), dom_depth(use));