From 799d87cbc513d04273cec246bb1f79a1325d647a Mon Sep 17 00:00:00 2001 From: MaxXSoft Date: Fri, 31 May 2024 15:20:45 +0800 Subject: [PATCH 01/13] Make `Node::dominates` more precise so that iterators of `ConcurrentHashMap` can be scalar replaced. --- src/hotspot/share/opto/memnode.cpp | 4 +- src/hotspot/share/opto/node.cpp | 80 ++++++++++++------- src/hotspot/share/opto/node.hpp | 6 +- .../bench/java/util/concurrent/Maps.java | 16 ++++ 4 files changed, 72 insertions(+), 34 deletions(-) diff --git a/src/hotspot/share/opto/memnode.cpp b/src/hotspot/share/opto/memnode.cpp index d0b6c59637f13..add5e223fbf16 100644 --- a/src/hotspot/share/opto/memnode.cpp +++ b/src/hotspot/share/opto/memnode.cpp @@ -472,7 +472,7 @@ bool MemNode::all_controls_dominate(Node* dom, Node* sub) { // Check all control edges of 'dom'. ResourceMark rm; - Node_List nlist; + Node_Stack nstack(0); Unique_Node_List dom_list; dom_list.push(dom); @@ -492,7 +492,7 @@ bool MemNode::all_controls_dominate(Node* dom, Node* sub) { } else if (n->is_Con() || n->is_Start() || n->is_Root()) { only_dominating_controls = true; } else if (n->is_CFG()) { - if (n->dominates(sub, nlist)) + if (n->dominates(sub, nstack)) only_dominating_controls = true; else return false; diff --git a/src/hotspot/share/opto/node.cpp b/src/hotspot/share/opto/node.cpp index d290541dffac3..56dbdfcb4a8f2 100644 --- a/src/hotspot/share/opto/node.cpp +++ b/src/hotspot/share/opto/node.cpp @@ -1249,7 +1249,7 @@ Node* Node::find_exact_control(Node* ctrl) { // We already know that if any path back to Root or Start reaches 'this', // then all paths so, so this is a simple search for one example, // not an exhaustive search for a counterexample. -bool Node::dominates(Node* sub, Node_List &nlist) { +bool Node::dominates(Node* sub, Node_Stack &nstack) { assert(this->is_CFG(), "expecting control"); assert(sub != nullptr && sub->is_CFG(), "expecting control"); @@ -1259,7 +1259,7 @@ bool Node::dominates(Node* sub, Node_List &nlist) { Node* orig_sub = sub; Node* dom = this; bool met_dom = false; - nlist.clear(); + nstack.clear(); // Walk 'sub' backward up the chain to 'dom', watching for regions. // After seeing 'dom', continue up to Root or Start. @@ -1269,9 +1269,29 @@ bool Node::dominates(Node* sub, Node_List &nlist) { // will either exit through the loop head, or give up. // (If we get confused, break out and return a conservative 'false'.) while (sub != nullptr) { - if (sub->is_top()) break; // Conservative answer for dead code. + if (sub->is_top()) { + // We reached a dead code, try another path of the last multi-input Region we met. + bool found = false; + for (uint i = nstack.size(); i > 0; i--) { + Node* n = nstack.node_at(i - 1); + for (uint index = nstack.index_at(i - 1); index < n->req(); index++) { + Node* in = n->in(index); + if (in != nullptr && !in->is_top()) { + found = true; + sub = n->find_exact_control(in); + nstack.set_index_at(i - 1, index + 1); + break; + } + } + if (found) break; + } + if (found) continue; + // Give a conservative answer if we have no other choices. + break; + } + if (sub == dom) { - if (nlist.size() == 0) { + if (nstack.is_empty()) { // No Region nodes except loops were visited before and the EntryControl // path was taken for loops: it did not walk in a cycle. return true; @@ -1282,7 +1302,7 @@ bool Node::dominates(Node* sub, Node_List &nlist) { // to make sure that it did not walk in a cycle. met_dom = true; // first time meet iterations_without_region_limit = DominatorSearchLimit; // Reset - } + } } if (sub->is_Start() || sub->is_Root()) { // Success if we met 'dom' along a path to Start or Root. @@ -1290,6 +1310,7 @@ bool Node::dominates(Node* sub, Node_List &nlist) { // (This assumption is up to the caller to ensure!) return met_dom; } + Node* up = sub->in(0); // Normalize simple pass-through regions and projections: up = sub->find_exact_control(up); @@ -1301,9 +1322,7 @@ bool Node::dominates(Node* sub, Node_List &nlist) { // Take in(1) path on the way up to 'dom' for regions with only one input up = sub->in(1); } else if (sub == up && sub->is_Region()) { - // Try both paths for Regions with 2 input paths (it may be a loop head). - // It could give conservative 'false' answer without information - // which region's input is the entry path. + // Try all paths for Regions with multiple input paths (it may be a loop head). iterations_without_region_limit = DominatorSearchLimit; // Reset bool region_was_visited_before = false; @@ -1312,40 +1331,38 @@ bool Node::dominates(Node* sub, Node_List &nlist) { // loop-back edge from 'sub' back into the body of the loop, // and worked our way up again to the loop header 'sub'. // So, take the first unexplored path on the way up to 'dom'. - for (int j = nlist.size() - 1; j >= 0; j--) { - intptr_t ni = (intptr_t)nlist.at(j); - Node* visited = (Node*)(ni & ~1); - bool visited_twice_already = ((ni & 1) != 0); + for (uint i = nstack.size(); i > 0; i--) { + Node* visited = nstack.node_at(i - 1); if (visited == sub) { - if (visited_twice_already) { - // Visited 2 paths, but still stuck in loop body. Give up. + // The Region node was visited before, just visit the next input. + for (uint index = nstack.index_at(i - 1); index < sub->req(); index++) { + Node* in = sub->in(index); + if (in != nullptr && !in->is_top() && in != sub) { + region_was_visited_before = true; + up = in; + nstack.set_index_at(i - 1, index + 1); + break; + } + } + if (!region_was_visited_before) { + // Visited all paths, but still stuck in loop body. Give up. return false; } - // The Region node was visited before only once. - // (We will repush with the low bit set, below.) - nlist.remove(j); - // We will find a new edge and re-insert. - region_was_visited_before = true; break; } } - // Find an incoming edge which has not been seen yet; walk through it. - assert(up == sub, ""); - uint skip = region_was_visited_before ? 1 : 0; - for (uint i = 1; i < sub->req(); i++) { - Node* in = sub->in(i); - if (in != nullptr && !in->is_top() && in != sub) { - if (skip == 0) { + // Record this Region if it's not visited before, and visit its first input. + if (!region_was_visited_before) { + for (uint i = 1; i < sub->req(); i++) { + Node* in = sub->in(i); + if (in != nullptr && !in->is_top() && in != sub) { up = in; + nstack.push(sub, i + 1); break; } - --skip; // skip this nontrivial input } } - - // Set 0 bit to indicate that both paths were taken. - nlist.push((Node*)((intptr_t)sub + (region_was_visited_before ? 1 : 0))); } if (up == sub) { @@ -1358,7 +1375,8 @@ bool Node::dominates(Node* sub, Node_List &nlist) { if (--iterations_without_region_limit < 0) { break; // dead cycle } - sub = up; + // Skip simple pass-through control nodes in chain. + sub = sub->find_exact_control(up); } // Did not meet Root or Start node in pred. chain. diff --git a/src/hotspot/share/opto/node.hpp b/src/hotspot/share/opto/node.hpp index ae379c4833a56..d70ed8aaa4181 100644 --- a/src/hotspot/share/opto/node.hpp +++ b/src/hotspot/share/opto/node.hpp @@ -1106,7 +1106,7 @@ class Node { Node* find_exact_control(Node* ctrl); // Check if 'this' node dominates or equal to 'sub'. - bool dominates(Node* sub, Node_List &nlist); + bool dominates(Node* sub, Node_Stack &nstack); protected: bool remove_dead_region(PhaseGVN *phase, bool can_reshape); @@ -1892,6 +1892,10 @@ class Node_Stack { void set_index(uint i) { _inode_top->indx = i; } + void set_index_at(uint i, uint index) { + assert(_inodes + i <= _inode_top, "in range"); + _inodes[i].indx = index; + } uint size_max() const { return (uint)pointer_delta(_inode_max, _inodes, sizeof(INode)); } // Max size uint size() const { return (uint)pointer_delta((_inode_top+1), _inodes, sizeof(INode)); } // Current size bool is_nonempty() const { return (_inode_top >= _inodes); } diff --git a/test/micro/org/openjdk/bench/java/util/concurrent/Maps.java b/test/micro/org/openjdk/bench/java/util/concurrent/Maps.java index bd68e582e6a2f..e8952ea2099cc 100644 --- a/test/micro/org/openjdk/bench/java/util/concurrent/Maps.java +++ b/test/micro/org/openjdk/bench/java/util/concurrent/Maps.java @@ -35,6 +35,7 @@ import org.openjdk.jmh.annotations.Threads; import org.openjdk.jmh.annotations.Warmup; +import java.util.Enumeration; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.TimeUnit; @@ -127,6 +128,21 @@ public ConcurrentHashMap testConcurrentHashMapPutAll() { return map; } + @Benchmark + public int testConcurrentHashMapIterators() { + ConcurrentHashMap map = (ConcurrentHashMap) staticMap; + int sum = 0; + Enumeration it = map.elements(); + while (it.hasMoreElements()) { + sum += (int) it.nextElement(); + } + it = map.keys(); + while (it.hasMoreElements()) { + sum += (int) it.nextElement(); + } + return sum; + } + private static class SimpleRandom { private final static long multiplier = 0x5DEECE66DL; private final static long addend = 0xBL; From c02de1c7c66afa29e58ed4f3a10b6c42c6156fcb Mon Sep 17 00:00:00 2001 From: MaxXSoft Date: Thu, 6 Jun 2024 14:54:37 +0800 Subject: [PATCH 02/13] Revert last commit, and push the `LoadNode` back to the worklist to wait for the dead code to be removed. --- src/hotspot/share/opto/memnode.cpp | 54 ++++++++++++++------ src/hotspot/share/opto/memnode.hpp | 2 +- src/hotspot/share/opto/node.cpp | 79 +++++++++++++----------------- src/hotspot/share/opto/node.hpp | 6 +-- 4 files changed, 73 insertions(+), 68 deletions(-) diff --git a/src/hotspot/share/opto/memnode.cpp b/src/hotspot/share/opto/memnode.cpp index add5e223fbf16..c481898806f85 100644 --- a/src/hotspot/share/opto/memnode.cpp +++ b/src/hotspot/share/opto/memnode.cpp @@ -427,14 +427,21 @@ Node *MemNode::Ideal_common(PhaseGVN *phase, bool can_reshape) { // Used by MemNode::find_previous_store to prove that the // control input of a memory operation predates (dominates) // an allocation it wants to look past. -bool MemNode::all_controls_dominate(Node* dom, Node* sub) { - if (dom == nullptr || dom->is_top() || sub == nullptr || sub->is_top()) +bool MemNode::all_controls_dominate(Node* dom, Node* sub, bool *dead_code) { + bool dummy_flag, &dead_code_flag = dead_code != nullptr ? *dead_code : dummy_flag; + dead_code_flag = false; + + if (dom == nullptr || dom->is_top() || sub == nullptr || sub->is_top()) { + dead_code_flag = true; return false; // Conservative answer for dead code + } // Check 'dom'. Skip Proj and CatchProj nodes. dom = dom->find_exact_control(dom); - if (dom == nullptr || dom->is_top()) + if (dom == nullptr || dom->is_top()) { + dead_code_flag = true; return false; // Conservative answer for dead code + } if (dom == sub) { // For the case when, for example, 'sub' is Initialize and the original @@ -457,8 +464,10 @@ bool MemNode::all_controls_dominate(Node* dom, Node* sub) { // Get control edge of 'sub'. Node* orig_sub = sub; sub = sub->find_exact_control(sub->in(0)); - if (sub == nullptr || sub->is_top()) + if (sub == nullptr || sub->is_top()) { + dead_code_flag = true; return false; // Conservative answer for dead code + } assert(sub->is_CFG(), "expecting control"); @@ -472,7 +481,7 @@ bool MemNode::all_controls_dominate(Node* dom, Node* sub) { // Check all control edges of 'dom'. ResourceMark rm; - Node_Stack nstack(0); + Node_List nlist; Unique_Node_List dom_list; dom_list.push(dom); @@ -485,23 +494,30 @@ bool MemNode::all_controls_dominate(Node* dom, Node* sub) { if (!n->is_CFG() && n->pinned()) { // Check only own control edge for pinned non-control nodes. n = n->find_exact_control(n->in(0)); - if (n == nullptr || n->is_top()) + if (n == nullptr || n->is_top()) { + dead_code_flag = true; return false; // Conservative answer for dead code + } assert(n->is_CFG(), "expecting control"); dom_list.push(n); } else if (n->is_Con() || n->is_Start() || n->is_Root()) { only_dominating_controls = true; } else if (n->is_CFG()) { - if (n->dominates(sub, nstack)) + bool dead_code; + if (n->dominates(sub, nlist, dead_code)) { only_dominating_controls = true; - else + } else { + if (dead_code) dead_code_flag = true; return false; + } } else { // First, own control edge. Node* m = n->find_exact_control(n->in(0)); if (m != nullptr) { - if (m->is_top()) + if (m->is_top()) { + dead_code_flag = true; return false; // Conservative answer for dead code + } dom_list.push(m); } // Now, the rest of edges. @@ -1658,35 +1674,41 @@ Node* LoadNode::split_through_phi(PhaseGVN* phase, bool ignore_missing_instance_ // Select Region to split through. Node* region; + bool dead_code, can_not_split = false; + PhaseIterGVN* igvn = phase->is_IterGVN(); if (!base_is_phi) { assert(mem->is_Phi(), "sanity"); region = mem->in(0); // Skip if the region dominates some control edge of the address. - if (!MemNode::all_controls_dominate(address, region)) - return nullptr; + if (!MemNode::all_controls_dominate(address, region, &dead_code)) + can_not_split = true; } else if (!mem->is_Phi()) { assert(base_is_phi, "sanity"); region = base->in(0); // Skip if the region dominates some control edge of the memory. - if (!MemNode::all_controls_dominate(mem, region)) - return nullptr; + if (!MemNode::all_controls_dominate(mem, region, &dead_code)) + can_not_split = true; } else if (base->in(0) != mem->in(0)) { assert(base_is_phi && mem->is_Phi(), "sanity"); if (MemNode::all_controls_dominate(mem, base->in(0))) { region = base->in(0); - } else if (MemNode::all_controls_dominate(address, mem->in(0))) { + } else if (MemNode::all_controls_dominate(address, mem->in(0), &dead_code)) { region = mem->in(0); } else { - return nullptr; // complex graph + can_not_split = true; // complex graph } } else { assert(base->in(0) == mem->in(0), "sanity"); region = mem->in(0); } + if (can_not_split) { + // Wait for the dead code to be removed. + if (dead_code) igvn->_worklist.push(this); + return nullptr; + } Node* phi = nullptr; const Type* this_type = this->bottom_type(); - PhaseIterGVN* igvn = phase->is_IterGVN(); if (t_oop != nullptr && (t_oop->is_known_instance_field() || load_boxed_values)) { int this_index = C->get_alias_index(t_oop); int this_offset = t_oop->offset(); diff --git a/src/hotspot/share/opto/memnode.hpp b/src/hotspot/share/opto/memnode.hpp index e0f5a4374133a..95f240d802041 100644 --- a/src/hotspot/share/opto/memnode.hpp +++ b/src/hotspot/share/opto/memnode.hpp @@ -106,7 +106,7 @@ class MemNode : public Node { static Node *optimize_simple_memory_chain(Node *mchain, const TypeOopPtr *t_oop, Node *load, PhaseGVN *phase); static Node *optimize_memory_chain(Node *mchain, const TypePtr *t_adr, Node *load, PhaseGVN *phase); // This one should probably be a phase-specific function: - static bool all_controls_dominate(Node* dom, Node* sub); + static bool all_controls_dominate(Node* dom, Node* sub, bool *dead_code = nullptr); virtual const class TypePtr *adr_type() const; // returns bottom_type of address diff --git a/src/hotspot/share/opto/node.cpp b/src/hotspot/share/opto/node.cpp index 56dbdfcb4a8f2..fe050ddaf733a 100644 --- a/src/hotspot/share/opto/node.cpp +++ b/src/hotspot/share/opto/node.cpp @@ -1249,7 +1249,7 @@ Node* Node::find_exact_control(Node* ctrl) { // We already know that if any path back to Root or Start reaches 'this', // then all paths so, so this is a simple search for one example, // not an exhaustive search for a counterexample. -bool Node::dominates(Node* sub, Node_Stack &nstack) { +bool Node::dominates(Node* sub, Node_List &nlist, bool &dead_code) { assert(this->is_CFG(), "expecting control"); assert(sub != nullptr && sub->is_CFG(), "expecting control"); @@ -1259,7 +1259,8 @@ bool Node::dominates(Node* sub, Node_Stack &nstack) { Node* orig_sub = sub; Node* dom = this; bool met_dom = false; - nstack.clear(); + dead_code = false; + nlist.clear(); // Walk 'sub' backward up the chain to 'dom', watching for regions. // After seeing 'dom', continue up to Root or Start. @@ -1270,28 +1271,12 @@ bool Node::dominates(Node* sub, Node_Stack &nstack) { // (If we get confused, break out and return a conservative 'false'.) while (sub != nullptr) { if (sub->is_top()) { - // We reached a dead code, try another path of the last multi-input Region we met. - bool found = false; - for (uint i = nstack.size(); i > 0; i--) { - Node* n = nstack.node_at(i - 1); - for (uint index = nstack.index_at(i - 1); index < n->req(); index++) { - Node* in = n->in(index); - if (in != nullptr && !in->is_top()) { - found = true; - sub = n->find_exact_control(in); - nstack.set_index_at(i - 1, index + 1); - break; - } - } - if (found) break; - } - if (found) continue; - // Give a conservative answer if we have no other choices. + // Conservative answer for dead code. + dead_code = true; break; } - if (sub == dom) { - if (nstack.is_empty()) { + if (nlist.size() == 0) { // No Region nodes except loops were visited before and the EntryControl // path was taken for loops: it did not walk in a cycle. return true; @@ -1302,7 +1287,7 @@ bool Node::dominates(Node* sub, Node_Stack &nstack) { // to make sure that it did not walk in a cycle. met_dom = true; // first time meet iterations_without_region_limit = DominatorSearchLimit; // Reset - } + } } if (sub->is_Start() || sub->is_Root()) { // Success if we met 'dom' along a path to Start or Root. @@ -1310,7 +1295,6 @@ bool Node::dominates(Node* sub, Node_Stack &nstack) { // (This assumption is up to the caller to ensure!) return met_dom; } - Node* up = sub->in(0); // Normalize simple pass-through regions and projections: up = sub->find_exact_control(up); @@ -1322,7 +1306,9 @@ bool Node::dominates(Node* sub, Node_Stack &nstack) { // Take in(1) path on the way up to 'dom' for regions with only one input up = sub->in(1); } else if (sub == up && sub->is_Region()) { - // Try all paths for Regions with multiple input paths (it may be a loop head). + // Try both paths for Regions with 2 input paths (it may be a loop head). + // It could give conservative 'false' answer without information + // which region's input is the entry path. iterations_without_region_limit = DominatorSearchLimit; // Reset bool region_was_visited_before = false; @@ -1331,38 +1317,40 @@ bool Node::dominates(Node* sub, Node_Stack &nstack) { // loop-back edge from 'sub' back into the body of the loop, // and worked our way up again to the loop header 'sub'. // So, take the first unexplored path on the way up to 'dom'. - for (uint i = nstack.size(); i > 0; i--) { - Node* visited = nstack.node_at(i - 1); + for (int j = nlist.size() - 1; j >= 0; j--) { + intptr_t ni = (intptr_t)nlist.at(j); + Node* visited = (Node*)(ni & ~1); + bool visited_twice_already = ((ni & 1) != 0); if (visited == sub) { - // The Region node was visited before, just visit the next input. - for (uint index = nstack.index_at(i - 1); index < sub->req(); index++) { - Node* in = sub->in(index); - if (in != nullptr && !in->is_top() && in != sub) { - region_was_visited_before = true; - up = in; - nstack.set_index_at(i - 1, index + 1); - break; - } - } - if (!region_was_visited_before) { - // Visited all paths, but still stuck in loop body. Give up. + if (visited_twice_already) { + // Visited 2 paths, but still stuck in loop body. Give up. return false; } + // The Region node was visited before only once. + // (We will repush with the low bit set, below.) + nlist.remove(j); + // We will find a new edge and re-insert. + region_was_visited_before = true; break; } } - // Record this Region if it's not visited before, and visit its first input. - if (!region_was_visited_before) { - for (uint i = 1; i < sub->req(); i++) { - Node* in = sub->in(i); - if (in != nullptr && !in->is_top() && in != sub) { + // Find an incoming edge which has not been seen yet; walk through it. + assert(up == sub, ""); + uint skip = region_was_visited_before ? 1 : 0; + for (uint i = 1; i < sub->req(); i++) { + Node* in = sub->in(i); + if (in != nullptr && !in->is_top() && in != sub) { + if (skip == 0) { up = in; - nstack.push(sub, i + 1); break; } + --skip; // skip this nontrivial input } } + + // Set 0 bit to indicate that both paths were taken. + nlist.push((Node*)((intptr_t)sub + (region_was_visited_before ? 1 : 0))); } if (up == sub) { @@ -1375,8 +1363,7 @@ bool Node::dominates(Node* sub, Node_Stack &nstack) { if (--iterations_without_region_limit < 0) { break; // dead cycle } - // Skip simple pass-through control nodes in chain. - sub = sub->find_exact_control(up); + sub = up; } // Did not meet Root or Start node in pred. chain. diff --git a/src/hotspot/share/opto/node.hpp b/src/hotspot/share/opto/node.hpp index d70ed8aaa4181..82fb68e7d56b5 100644 --- a/src/hotspot/share/opto/node.hpp +++ b/src/hotspot/share/opto/node.hpp @@ -1106,7 +1106,7 @@ class Node { Node* find_exact_control(Node* ctrl); // Check if 'this' node dominates or equal to 'sub'. - bool dominates(Node* sub, Node_Stack &nstack); + bool dominates(Node* sub, Node_List &nlist, bool &dead_code); protected: bool remove_dead_region(PhaseGVN *phase, bool can_reshape); @@ -1892,10 +1892,6 @@ class Node_Stack { void set_index(uint i) { _inode_top->indx = i; } - void set_index_at(uint i, uint index) { - assert(_inodes + i <= _inode_top, "in range"); - _inodes[i].indx = index; - } uint size_max() const { return (uint)pointer_delta(_inode_max, _inodes, sizeof(INode)); } // Max size uint size() const { return (uint)pointer_delta((_inode_top+1), _inodes, sizeof(INode)); } // Current size bool is_nonempty() const { return (_inode_top >= _inodes); } From 5c09678fe3814ac102e0ed857e8fb5d7fba08255 Mon Sep 17 00:00:00 2001 From: MaxXSoft Date: Tue, 11 Jun 2024 10:20:12 +0800 Subject: [PATCH 03/13] Add IR test and update copyright. --- .../ScalarReplacementWithGCBarrierTests.java | 115 ++++++++++++++++++ .../bench/java/util/concurrent/Maps.java | 2 +- 2 files changed, 116 insertions(+), 1 deletion(-) create mode 100644 test/hotspot/jtreg/compiler/c2/irTests/scalarReplacement/ScalarReplacementWithGCBarrierTests.java diff --git a/test/hotspot/jtreg/compiler/c2/irTests/scalarReplacement/ScalarReplacementWithGCBarrierTests.java b/test/hotspot/jtreg/compiler/c2/irTests/scalarReplacement/ScalarReplacementWithGCBarrierTests.java new file mode 100644 index 0000000000000..4ab0a95df8b18 --- /dev/null +++ b/test/hotspot/jtreg/compiler/c2/irTests/scalarReplacement/ScalarReplacementWithGCBarrierTests.java @@ -0,0 +1,115 @@ +/* + * Copyright (c) 2024 Alibaba Group Holding Limited. 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. + */ + +package compiler.c2.irTests.scalarReplacement; + +import compiler.lib.ir_framework.*; + +/* + * @test + * @bug 8333334 + * @summary Tests that dead barrier control flows do not affect the scalar replacement. + * @library /test/lib / + * @requires vm.compiler2.enabled + * @run driver compiler.c2.irTests.scalarReplacement.ScalarReplacementWithGCBarrierTests + */ +public class ScalarReplacementWithGCBarrierTests { + static class List { + public Node head; + + public void push(int value) { + Node n = new Node(); + n.value = value; + n.next = head; + head = n; + } + + public Iter iter() { + Iter iter = new Iter(); + iter.list = this; + iter.n = head; + iter.sum = 0; + return iter; + } + } + + static class Node { + public int value; + public Node next; + } + + static class Iter { + public List list; + public Node n; + public Integer sum; + + public boolean next() { + int lastSum = sum; + while (sum - lastSum < 1000) { + while (n != null && n.value < 30) n = n.next; + if (n == null) return false; + sum += n.value; + n = n.next; + } + return true; + } + } + + private static final int SIZE = 1000; + + public static void main(String[] args) { + // Must use G1 GC to ensure there is a pre-barrier + // before the first field write. + TestFramework.runWithFlags("-XX:+UseG1GC"); + } + + @Run(test = "testScalarReplacementWithGCBarrier") + private void runner() { + List list = new List(); + for (int i = 0; i < SIZE; i++) { + list.push(i); + } + testScalarReplacementWithGCBarrier(list); + } + + // Allocation of `Iter iter` should be eliminated by scalar replacement, and + // the allocation of `Integer sum` can not be eliminated, so there should be + // 1 allocation after allocations and locks elimination. + // + // Before the patch of JDK-8333334, both allocations of `Iter` and `Integer` + // could not be eliminated. + @Test + @IR(phase = { CompilePhase.AFTER_PARSING }, counts = { IRNode.ALLOC, "1" }) + @IR(phase = { CompilePhase.INCREMENTAL_BOXING_INLINE }, counts = { IRNode.ALLOC, "2" }) + @IR(applyIf = { "UseG1GC", "true" }, phase = { CompilePhase.ITER_GVN_AFTER_ELIMINATION }, counts = { IRNode.ALLOC, "1" }) + private int testScalarReplacementWithGCBarrier(List list) { + Iter iter = list.iter(); + for (;;) { + while (iter.next()) {} + if (list.head == null) break; + list.head = list.head.next; + iter.n = list.head; + } + return iter.sum; + } +} diff --git a/test/micro/org/openjdk/bench/java/util/concurrent/Maps.java b/test/micro/org/openjdk/bench/java/util/concurrent/Maps.java index e8952ea2099cc..d6f3b587dfa72 100644 --- a/test/micro/org/openjdk/bench/java/util/concurrent/Maps.java +++ b/test/micro/org/openjdk/bench/java/util/concurrent/Maps.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2014, 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 From d56e0372a8bdb79c1f71de6a54008b0487a7d26d Mon Sep 17 00:00:00 2001 From: MaxXSoft Date: Tue, 25 Jun 2024 11:32:20 +0800 Subject: [PATCH 04/13] Add `@ForceInline` to the IR test. --- .../scalarReplacement/ScalarReplacementWithGCBarrierTests.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/hotspot/jtreg/compiler/c2/irTests/scalarReplacement/ScalarReplacementWithGCBarrierTests.java b/test/hotspot/jtreg/compiler/c2/irTests/scalarReplacement/ScalarReplacementWithGCBarrierTests.java index 4ab0a95df8b18..039e4c639af12 100644 --- a/test/hotspot/jtreg/compiler/c2/irTests/scalarReplacement/ScalarReplacementWithGCBarrierTests.java +++ b/test/hotspot/jtreg/compiler/c2/irTests/scalarReplacement/ScalarReplacementWithGCBarrierTests.java @@ -44,6 +44,7 @@ public void push(int value) { head = n; } + @ForceInline public Iter iter() { Iter iter = new Iter(); iter.list = this; @@ -63,6 +64,7 @@ static class Iter { public Node n; public Integer sum; + @ForceInline public boolean next() { int lastSum = sum; while (sum - lastSum < 1000) { From f309ea41f1e13310dab1e84b6b0f3fcb9472c474 Mon Sep 17 00:00:00 2001 From: MaxXSoft Date: Mon, 1 Jul 2024 15:48:00 +0800 Subject: [PATCH 05/13] Add `@requires` for G1 GC. --- .../scalarReplacement/ScalarReplacementWithGCBarrierTests.java | 1 + 1 file changed, 1 insertion(+) diff --git a/test/hotspot/jtreg/compiler/c2/irTests/scalarReplacement/ScalarReplacementWithGCBarrierTests.java b/test/hotspot/jtreg/compiler/c2/irTests/scalarReplacement/ScalarReplacementWithGCBarrierTests.java index 039e4c639af12..6630fcdfa2bbd 100644 --- a/test/hotspot/jtreg/compiler/c2/irTests/scalarReplacement/ScalarReplacementWithGCBarrierTests.java +++ b/test/hotspot/jtreg/compiler/c2/irTests/scalarReplacement/ScalarReplacementWithGCBarrierTests.java @@ -31,6 +31,7 @@ * @summary Tests that dead barrier control flows do not affect the scalar replacement. * @library /test/lib / * @requires vm.compiler2.enabled + * @requires vm.gc.G1 * @run driver compiler.c2.irTests.scalarReplacement.ScalarReplacementWithGCBarrierTests */ public class ScalarReplacementWithGCBarrierTests { From 38418f26120a5e51f39f1e16254719710f77bd7b Mon Sep 17 00:00:00 2001 From: MaxXSoft Date: Mon, 8 Jul 2024 14:43:11 +0800 Subject: [PATCH 06/13] Make `Node::dominates` and `MemNode::all_controls_dominate` returns an `enum`. --- src/hotspot/share/opto/memnode.cpp | 100 +++++++++++++---------------- src/hotspot/share/opto/memnode.hpp | 2 +- src/hotspot/share/opto/node.cpp | 15 ++--- src/hotspot/share/opto/node.hpp | 8 ++- 4 files changed, 59 insertions(+), 66 deletions(-) diff --git a/src/hotspot/share/opto/memnode.cpp b/src/hotspot/share/opto/memnode.cpp index c481898806f85..32c0994aa0407 100644 --- a/src/hotspot/share/opto/memnode.cpp +++ b/src/hotspot/share/opto/memnode.cpp @@ -427,30 +427,23 @@ Node *MemNode::Ideal_common(PhaseGVN *phase, bool can_reshape) { // Used by MemNode::find_previous_store to prove that the // control input of a memory operation predates (dominates) // an allocation it wants to look past. -bool MemNode::all_controls_dominate(Node* dom, Node* sub, bool *dead_code) { - bool dummy_flag, &dead_code_flag = dead_code != nullptr ? *dead_code : dummy_flag; - dead_code_flag = false; - - if (dom == nullptr || dom->is_top() || sub == nullptr || sub->is_top()) { - dead_code_flag = true; - return false; // Conservative answer for dead code - } +Node::DomResult MemNode::all_controls_dominate(Node* dom, Node* sub) { + if (dom == nullptr || dom->is_top() || sub == nullptr || sub->is_top()) + return DomResult::EncounteredDeadCode; // Conservative answer for dead code // Check 'dom'. Skip Proj and CatchProj nodes. dom = dom->find_exact_control(dom); - if (dom == nullptr || dom->is_top()) { - dead_code_flag = true; - return false; // Conservative answer for dead code - } + if (dom == nullptr || dom->is_top()) + return DomResult::EncounteredDeadCode; // Conservative answer for dead code if (dom == sub) { // For the case when, for example, 'sub' is Initialize and the original // 'dom' is Proj node of the 'sub'. - return false; + return DomResult::NotDominate; } if (dom->is_Con() || dom->is_Start() || dom->is_Root() || dom == sub) - return true; + return DomResult::Dominate; // 'dom' dominates 'sub' if its control edge and control edges // of all its inputs dominate or equal to sub's control edge. @@ -464,18 +457,16 @@ bool MemNode::all_controls_dominate(Node* dom, Node* sub, bool *dead_code) { // Get control edge of 'sub'. Node* orig_sub = sub; sub = sub->find_exact_control(sub->in(0)); - if (sub == nullptr || sub->is_top()) { - dead_code_flag = true; - return false; // Conservative answer for dead code - } + if (sub == nullptr || sub->is_top()) + return DomResult::EncounteredDeadCode; // Conservative answer for dead code assert(sub->is_CFG(), "expecting control"); if (sub == dom) - return true; + return DomResult::Dominate; if (sub->is_Start() || sub->is_Root()) - return false; + return DomResult::NotDominate; { // Check all control edges of 'dom'. @@ -490,34 +481,28 @@ bool MemNode::all_controls_dominate(Node* dom, Node* sub, bool *dead_code) { for (uint next = 0; next < dom_list.size(); next++) { Node* n = dom_list.at(next); if (n == orig_sub) - return false; // One of dom's inputs dominated by sub. + return DomResult::NotDominate; // One of dom's inputs dominated by sub. if (!n->is_CFG() && n->pinned()) { // Check only own control edge for pinned non-control nodes. n = n->find_exact_control(n->in(0)); - if (n == nullptr || n->is_top()) { - dead_code_flag = true; - return false; // Conservative answer for dead code - } + if (n == nullptr || n->is_top()) + return DomResult::EncounteredDeadCode; // Conservative answer for dead code assert(n->is_CFG(), "expecting control"); dom_list.push(n); } else if (n->is_Con() || n->is_Start() || n->is_Root()) { only_dominating_controls = true; } else if (n->is_CFG()) { - bool dead_code; - if (n->dominates(sub, nlist, dead_code)) { + DomResult dom_result = n->dominates(sub, nlist); + if (dom_result == DomResult::Dominate) only_dominating_controls = true; - } else { - if (dead_code) dead_code_flag = true; - return false; - } + else + return dom_result; } else { // First, own control edge. Node* m = n->find_exact_control(n->in(0)); if (m != nullptr) { - if (m->is_top()) { - dead_code_flag = true; - return false; // Conservative answer for dead code - } + if (m->is_top()) + return DomResult::EncounteredDeadCode; // Conservative answer for dead code dom_list.push(m); } // Now, the rest of edges. @@ -530,7 +515,7 @@ bool MemNode::all_controls_dominate(Node* dom, Node* sub, bool *dead_code) { } } } - return only_dominating_controls; + return only_dominating_controls ? DomResult::Dominate : DomResult::NotDominate; } } @@ -552,9 +537,9 @@ bool MemNode::detect_ptr_independence(Node* p1, AllocateNode* a1, return (a1 != a2); } else if (a1 != nullptr) { // one allocation a1 // (Note: p2->is_Con implies p2->in(0)->is_Root, which dominates.) - return all_controls_dominate(p2, a1); + return all_controls_dominate(p2, a1) == DomResult::Dominate; } else { //(a2 != null) // one allocation a2 - return all_controls_dominate(p1, a2); + return all_controls_dominate(p1, a2) == DomResult::Dominate; } return false; } @@ -750,7 +735,7 @@ Node* MemNode::find_previous_store(PhaseValues* phase) { known_identical = true; else if (alloc != nullptr) known_independent = true; - else if (all_controls_dominate(this, st_alloc)) + else if (all_controls_dominate(this, st_alloc) == DomResult::Dominate) known_independent = true; if (known_independent) { @@ -1582,10 +1567,10 @@ bool LoadNode::can_split_through_phi_base(PhaseGVN* phase) { } if (!mem->is_Phi()) { - if (!MemNode::all_controls_dominate(mem, base->in(0))) + if (MemNode::all_controls_dominate(mem, base->in(0)) != DomResult::Dominate) return false; } else if (base->in(0) != mem->in(0)) { - if (!MemNode::all_controls_dominate(mem, base->in(0))) { + if (MemNode::all_controls_dominate(mem, base->in(0)) != DomResult::Dominate) { return false; } } @@ -1674,36 +1659,41 @@ Node* LoadNode::split_through_phi(PhaseGVN* phase, bool ignore_missing_instance_ // Select Region to split through. Node* region; - bool dead_code, can_not_split = false; + DomResult dom_result = DomResult::Dominate; PhaseIterGVN* igvn = phase->is_IterGVN(); if (!base_is_phi) { assert(mem->is_Phi(), "sanity"); region = mem->in(0); // Skip if the region dominates some control edge of the address. - if (!MemNode::all_controls_dominate(address, region, &dead_code)) - can_not_split = true; + // We will check `dom_result` later. + dom_result = MemNode::all_controls_dominate(address, region); } else if (!mem->is_Phi()) { assert(base_is_phi, "sanity"); region = base->in(0); // Skip if the region dominates some control edge of the memory. - if (!MemNode::all_controls_dominate(mem, region, &dead_code)) - can_not_split = true; + // We will check `dom_result` later. + dom_result = MemNode::all_controls_dominate(mem, region); } else if (base->in(0) != mem->in(0)) { assert(base_is_phi && mem->is_Phi(), "sanity"); - if (MemNode::all_controls_dominate(mem, base->in(0))) { + dom_result = MemNode::all_controls_dominate(mem, base->in(0)); + if (dom_result == DomResult::Dominate) { region = base->in(0); - } else if (MemNode::all_controls_dominate(address, mem->in(0), &dead_code)) { - region = mem->in(0); } else { - can_not_split = true; // complex graph + dom_result = MemNode::all_controls_dominate(address, mem->in(0)); + if (dom_result == DomResult::Dominate) { + region = mem->in(0); + } + // Otherwise we encounter a complex graph. } } else { assert(base->in(0) == mem->in(0), "sanity"); region = mem->in(0); } - if (can_not_split) { - // Wait for the dead code to be removed. - if (dead_code) igvn->_worklist.push(this); + if (dom_result != DomResult::Dominate) { + if (dom_result == DomResult::EncounteredDeadCode) { + // Wait for the dead code to be removed. + igvn->_worklist.push(this); + } return nullptr; } @@ -1854,7 +1844,7 @@ Node *LoadNode::Ideal(PhaseGVN *phase, bool can_reshape) { if (in(MemNode::Control) != nullptr && can_remove_control() && phase->type(base)->higher_equal(TypePtr::NOTNULL) - && all_controls_dominate(base, phase->C->start())) { + && all_controls_dominate(base, phase->C->start()) == DomResult::Dominate) { // A method-invariant, non-null address (constant or 'this' argument). set_req(MemNode::Control, nullptr); progress = true; @@ -4588,7 +4578,7 @@ bool InitializeNode::detect_init_independence(Node* value, PhaseGVN* phase) { // must have preceded the init, or else be equal to the init. // Even after loop optimizations (which might change control edges) // a store is never pinned *before* the availability of its inputs. - if (!MemNode::all_controls_dominate(n, this)) + if (MemNode::all_controls_dominate(n, this) != DomResult::Dominate) return false; // failed to prove a good control } diff --git a/src/hotspot/share/opto/memnode.hpp b/src/hotspot/share/opto/memnode.hpp index 95f240d802041..46a15cfbdc906 100644 --- a/src/hotspot/share/opto/memnode.hpp +++ b/src/hotspot/share/opto/memnode.hpp @@ -106,7 +106,7 @@ class MemNode : public Node { static Node *optimize_simple_memory_chain(Node *mchain, const TypeOopPtr *t_oop, Node *load, PhaseGVN *phase); static Node *optimize_memory_chain(Node *mchain, const TypePtr *t_adr, Node *load, PhaseGVN *phase); // This one should probably be a phase-specific function: - static bool all_controls_dominate(Node* dom, Node* sub, bool *dead_code = nullptr); + static DomResult all_controls_dominate(Node* dom, Node* sub); virtual const class TypePtr *adr_type() const; // returns bottom_type of address diff --git a/src/hotspot/share/opto/node.cpp b/src/hotspot/share/opto/node.cpp index fe050ddaf733a..b8ea163dfac14 100644 --- a/src/hotspot/share/opto/node.cpp +++ b/src/hotspot/share/opto/node.cpp @@ -1249,7 +1249,7 @@ Node* Node::find_exact_control(Node* ctrl) { // We already know that if any path back to Root or Start reaches 'this', // then all paths so, so this is a simple search for one example, // not an exhaustive search for a counterexample. -bool Node::dominates(Node* sub, Node_List &nlist, bool &dead_code) { +Node::DomResult Node::dominates(Node* sub, Node_List &nlist) { assert(this->is_CFG(), "expecting control"); assert(sub != nullptr && sub->is_CFG(), "expecting control"); @@ -1259,7 +1259,6 @@ bool Node::dominates(Node* sub, Node_List &nlist, bool &dead_code) { Node* orig_sub = sub; Node* dom = this; bool met_dom = false; - dead_code = false; nlist.clear(); // Walk 'sub' backward up the chain to 'dom', watching for regions. @@ -1272,14 +1271,13 @@ bool Node::dominates(Node* sub, Node_List &nlist, bool &dead_code) { while (sub != nullptr) { if (sub->is_top()) { // Conservative answer for dead code. - dead_code = true; - break; + return DomResult::EncounteredDeadCode; } if (sub == dom) { if (nlist.size() == 0) { // No Region nodes except loops were visited before and the EntryControl // path was taken for loops: it did not walk in a cycle. - return true; + return DomResult::Dominate; } else if (met_dom) { break; // already met before: walk in a cycle } else { @@ -1293,7 +1291,7 @@ bool Node::dominates(Node* sub, Node_List &nlist, bool &dead_code) { // Success if we met 'dom' along a path to Start or Root. // We assume there are no alternative paths that avoid 'dom'. // (This assumption is up to the caller to ensure!) - return met_dom; + return met_dom ? DomResult::Dominate : DomResult::NotDominate; } Node* up = sub->in(0); // Normalize simple pass-through regions and projections: @@ -1324,7 +1322,7 @@ bool Node::dominates(Node* sub, Node_List &nlist, bool &dead_code) { if (visited == sub) { if (visited_twice_already) { // Visited 2 paths, but still stuck in loop body. Give up. - return false; + return DomResult::NotDominate; } // The Region node was visited before only once. // (We will repush with the low bit set, below.) @@ -1367,8 +1365,7 @@ bool Node::dominates(Node* sub, Node_List &nlist, bool &dead_code) { } // Did not meet Root or Start node in pred. chain. - // Conservative answer for dead code. - return false; + return DomResult::NotDominate; } //------------------------------remove_dead_region----------------------------- diff --git a/src/hotspot/share/opto/node.hpp b/src/hotspot/share/opto/node.hpp index 82fb68e7d56b5..076415a178d42 100644 --- a/src/hotspot/share/opto/node.hpp +++ b/src/hotspot/share/opto/node.hpp @@ -1105,8 +1105,14 @@ class Node { // Skip Proj and CatchProj nodes chains. Check for Null and Top. Node* find_exact_control(Node* ctrl); + // Results of the dominance analysis. + enum class DomResult { + NotDominate, // 'this' node does not dominate 'sub'. + Dominate, // 'this' node dominates or is equal to 'sub'. + EncounteredDeadCode, // Result is undefined due to encountering dead code. + }; // Check if 'this' node dominates or equal to 'sub'. - bool dominates(Node* sub, Node_List &nlist, bool &dead_code); + DomResult dominates(Node* sub, Node_List &nlist); protected: bool remove_dead_region(PhaseGVN *phase, bool can_reshape); From 35e7a0d8e401cb2744f970312df01b5ec0a5bb4d Mon Sep 17 00:00:00 2001 From: MaxXSoft Date: Tue, 9 Jul 2024 11:06:24 +0800 Subject: [PATCH 07/13] Add copyright. --- src/hotspot/share/opto/memnode.cpp | 1 + src/hotspot/share/opto/memnode.hpp | 1 + src/hotspot/share/opto/node.cpp | 1 + src/hotspot/share/opto/node.hpp | 1 + test/micro/org/openjdk/bench/java/util/concurrent/Maps.java | 3 ++- 5 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/hotspot/share/opto/memnode.cpp b/src/hotspot/share/opto/memnode.cpp index 32c0994aa0407..437e283dd33c1 100644 --- a/src/hotspot/share/opto/memnode.cpp +++ b/src/hotspot/share/opto/memnode.cpp @@ -1,5 +1,6 @@ /* * Copyright (c) 1997, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2024, Alibaba Group Holding Limited. 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 diff --git a/src/hotspot/share/opto/memnode.hpp b/src/hotspot/share/opto/memnode.hpp index 46a15cfbdc906..e3455a81402e4 100644 --- a/src/hotspot/share/opto/memnode.hpp +++ b/src/hotspot/share/opto/memnode.hpp @@ -1,5 +1,6 @@ /* * Copyright (c) 1997, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2024, Alibaba Group Holding Limited. 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 diff --git a/src/hotspot/share/opto/node.cpp b/src/hotspot/share/opto/node.cpp index b8ea163dfac14..fa317f42f571d 100644 --- a/src/hotspot/share/opto/node.cpp +++ b/src/hotspot/share/opto/node.cpp @@ -1,5 +1,6 @@ /* * Copyright (c) 1997, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2024, Alibaba Group Holding Limited. 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 diff --git a/src/hotspot/share/opto/node.hpp b/src/hotspot/share/opto/node.hpp index 076415a178d42..c7b1020a08bd7 100644 --- a/src/hotspot/share/opto/node.hpp +++ b/src/hotspot/share/opto/node.hpp @@ -1,5 +1,6 @@ /* * Copyright (c) 1997, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2024, Alibaba Group Holding Limited. 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 diff --git a/test/micro/org/openjdk/bench/java/util/concurrent/Maps.java b/test/micro/org/openjdk/bench/java/util/concurrent/Maps.java index d6f3b587dfa72..62883efb8bb89 100644 --- a/test/micro/org/openjdk/bench/java/util/concurrent/Maps.java +++ b/test/micro/org/openjdk/bench/java/util/concurrent/Maps.java @@ -1,5 +1,6 @@ /* - * Copyright (c) 2014, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2014, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2024, Alibaba Group Holding Limited. 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 From 785c06c85de9a78313e02883bff54a0e080b2c8d Mon Sep 17 00:00:00 2001 From: MaxXSoft Date: Thu, 22 Aug 2024 15:30:02 +0800 Subject: [PATCH 08/13] Add brackets around modified if/else branches. --- src/hotspot/share/opto/memnode.cpp | 51 +++++++++++++++++++----------- 1 file changed, 33 insertions(+), 18 deletions(-) diff --git a/src/hotspot/share/opto/memnode.cpp b/src/hotspot/share/opto/memnode.cpp index 437e283dd33c1..ac9b89acb3498 100644 --- a/src/hotspot/share/opto/memnode.cpp +++ b/src/hotspot/share/opto/memnode.cpp @@ -429,13 +429,15 @@ Node *MemNode::Ideal_common(PhaseGVN *phase, bool can_reshape) { // control input of a memory operation predates (dominates) // an allocation it wants to look past. Node::DomResult MemNode::all_controls_dominate(Node* dom, Node* sub) { - if (dom == nullptr || dom->is_top() || sub == nullptr || sub->is_top()) + if (dom == nullptr || dom->is_top() || sub == nullptr || sub->is_top()) { return DomResult::EncounteredDeadCode; // Conservative answer for dead code + } // Check 'dom'. Skip Proj and CatchProj nodes. dom = dom->find_exact_control(dom); - if (dom == nullptr || dom->is_top()) + if (dom == nullptr || dom->is_top()) { return DomResult::EncounteredDeadCode; // Conservative answer for dead code + } if (dom == sub) { // For the case when, for example, 'sub' is Initialize and the original @@ -443,8 +445,9 @@ Node::DomResult MemNode::all_controls_dominate(Node* dom, Node* sub) { return DomResult::NotDominate; } - if (dom->is_Con() || dom->is_Start() || dom->is_Root() || dom == sub) + if (dom->is_Con() || dom->is_Start() || dom->is_Root() || dom == sub) { return DomResult::Dominate; + } // 'dom' dominates 'sub' if its control edge and control edges // of all its inputs dominate or equal to sub's control edge. @@ -458,16 +461,19 @@ Node::DomResult MemNode::all_controls_dominate(Node* dom, Node* sub) { // Get control edge of 'sub'. Node* orig_sub = sub; sub = sub->find_exact_control(sub->in(0)); - if (sub == nullptr || sub->is_top()) + if (sub == nullptr || sub->is_top()) { return DomResult::EncounteredDeadCode; // Conservative answer for dead code + } assert(sub->is_CFG(), "expecting control"); - if (sub == dom) + if (sub == dom) { return DomResult::Dominate; + } - if (sub->is_Start() || sub->is_Root()) + if (sub->is_Start() || sub->is_Root()) { return DomResult::NotDominate; + } { // Check all control edges of 'dom'. @@ -481,37 +487,42 @@ Node::DomResult MemNode::all_controls_dominate(Node* dom, Node* sub) { for (uint next = 0; next < dom_list.size(); next++) { Node* n = dom_list.at(next); - if (n == orig_sub) + if (n == orig_sub) { return DomResult::NotDominate; // One of dom's inputs dominated by sub. + } if (!n->is_CFG() && n->pinned()) { // Check only own control edge for pinned non-control nodes. n = n->find_exact_control(n->in(0)); - if (n == nullptr || n->is_top()) + if (n == nullptr || n->is_top()) { return DomResult::EncounteredDeadCode; // Conservative answer for dead code + } assert(n->is_CFG(), "expecting control"); dom_list.push(n); } else if (n->is_Con() || n->is_Start() || n->is_Root()) { only_dominating_controls = true; } else if (n->is_CFG()) { DomResult dom_result = n->dominates(sub, nlist); - if (dom_result == DomResult::Dominate) + if (dom_result == DomResult::Dominate) { only_dominating_controls = true; - else + } else { return dom_result; + } } else { // First, own control edge. Node* m = n->find_exact_control(n->in(0)); if (m != nullptr) { - if (m->is_top()) + if (m->is_top()) { return DomResult::EncounteredDeadCode; // Conservative answer for dead code + } dom_list.push(m); } // Now, the rest of edges. uint cnt = n->req(); for (uint i = 1; i < cnt; i++) { m = n->find_exact_control(n->in(i)); - if (m == nullptr || m->is_top()) + if (m == nullptr || m->is_top()) { continue; + } dom_list.push(m); } } @@ -728,16 +739,18 @@ Node* MemNode::find_previous_store(PhaseValues* phase) { } else if (mem->is_Proj() && mem->in(0)->is_Initialize()) { InitializeNode* st_init = mem->in(0)->as_Initialize(); AllocateNode* st_alloc = st_init->allocation(); - if (st_alloc == nullptr) + if (st_alloc == nullptr) { break; // something degenerated + } bool known_identical = false; bool known_independent = false; - if (alloc == st_alloc) + if (alloc == st_alloc) { known_identical = true; - else if (alloc != nullptr) + } else if (alloc != nullptr) { known_independent = true; - else if (all_controls_dominate(this, st_alloc) == DomResult::Dominate) + } else if (all_controls_dominate(this, st_alloc) == DomResult::Dominate) { known_independent = true; + } if (known_independent) { // The bases are provably independent: Either they are @@ -1568,8 +1581,9 @@ bool LoadNode::can_split_through_phi_base(PhaseGVN* phase) { } if (!mem->is_Phi()) { - if (MemNode::all_controls_dominate(mem, base->in(0)) != DomResult::Dominate) + if (MemNode::all_controls_dominate(mem, base->in(0)) != DomResult::Dominate) { return false; + } } else if (base->in(0) != mem->in(0)) { if (MemNode::all_controls_dominate(mem, base->in(0)) != DomResult::Dominate) { return false; @@ -4579,8 +4593,9 @@ bool InitializeNode::detect_init_independence(Node* value, PhaseGVN* phase) { // must have preceded the init, or else be equal to the init. // Even after loop optimizations (which might change control edges) // a store is never pinned *before* the availability of its inputs. - if (MemNode::all_controls_dominate(n, this) != DomResult::Dominate) + if (MemNode::all_controls_dominate(n, this) != DomResult::Dominate) { return false; // failed to prove a good control + } } // Check data edges for possible dependencies on 'this'. From fb7c9204a3dea1fb966407cc5fda504e43bccb89 Mon Sep 17 00:00:00 2001 From: MaxXSoft Date: Thu, 22 Aug 2024 15:41:08 +0800 Subject: [PATCH 09/13] Fix style. --- src/hotspot/share/opto/memnode.cpp | 5 +++-- src/hotspot/share/opto/node.hpp | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/hotspot/share/opto/memnode.cpp b/src/hotspot/share/opto/memnode.cpp index ac9b89acb3498..9e9b2f7205342 100644 --- a/src/hotspot/share/opto/memnode.cpp +++ b/src/hotspot/share/opto/memnode.cpp @@ -1675,7 +1675,6 @@ Node* LoadNode::split_through_phi(PhaseGVN* phase, bool ignore_missing_instance_ // Select Region to split through. Node* region; DomResult dom_result = DomResult::Dominate; - PhaseIterGVN* igvn = phase->is_IterGVN(); if (!base_is_phi) { assert(mem->is_Phi(), "sanity"); region = mem->in(0); @@ -1698,12 +1697,14 @@ Node* LoadNode::split_through_phi(PhaseGVN* phase, bool ignore_missing_instance_ if (dom_result == DomResult::Dominate) { region = mem->in(0); } - // Otherwise we encounter a complex graph. + // Otherwise we encountered a complex graph. } } else { assert(base->in(0) == mem->in(0), "sanity"); region = mem->in(0); } + + PhaseIterGVN* igvn = phase->is_IterGVN(); if (dom_result != DomResult::Dominate) { if (dom_result == DomResult::EncounteredDeadCode) { // Wait for the dead code to be removed. diff --git a/src/hotspot/share/opto/node.hpp b/src/hotspot/share/opto/node.hpp index c7b1020a08bd7..4c83222d15a81 100644 --- a/src/hotspot/share/opto/node.hpp +++ b/src/hotspot/share/opto/node.hpp @@ -1110,7 +1110,7 @@ class Node { enum class DomResult { NotDominate, // 'this' node does not dominate 'sub'. Dominate, // 'this' node dominates or is equal to 'sub'. - EncounteredDeadCode, // Result is undefined due to encountering dead code. + EncounteredDeadCode // Result is undefined due to encountering dead code. }; // Check if 'this' node dominates or equal to 'sub'. DomResult dominates(Node* sub, Node_List &nlist); From 31bdcdb81d2bb3c214e8dbbb593ed23d002f7c21 Mon Sep 17 00:00:00 2001 From: MaxXSoft Date: Thu, 22 Aug 2024 15:45:02 +0800 Subject: [PATCH 10/13] Remove redundant `applyIf` and fix style for IR test. --- .../ScalarReplacementWithGCBarrierTests.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/hotspot/jtreg/compiler/c2/irTests/scalarReplacement/ScalarReplacementWithGCBarrierTests.java b/test/hotspot/jtreg/compiler/c2/irTests/scalarReplacement/ScalarReplacementWithGCBarrierTests.java index 6630fcdfa2bbd..fbf5cdd61cc03 100644 --- a/test/hotspot/jtreg/compiler/c2/irTests/scalarReplacement/ScalarReplacementWithGCBarrierTests.java +++ b/test/hotspot/jtreg/compiler/c2/irTests/scalarReplacement/ScalarReplacementWithGCBarrierTests.java @@ -104,10 +104,10 @@ private void runner() { @Test @IR(phase = { CompilePhase.AFTER_PARSING }, counts = { IRNode.ALLOC, "1" }) @IR(phase = { CompilePhase.INCREMENTAL_BOXING_INLINE }, counts = { IRNode.ALLOC, "2" }) - @IR(applyIf = { "UseG1GC", "true" }, phase = { CompilePhase.ITER_GVN_AFTER_ELIMINATION }, counts = { IRNode.ALLOC, "1" }) + @IR(phase = { CompilePhase.ITER_GVN_AFTER_ELIMINATION }, counts = { IRNode.ALLOC, "1" }) private int testScalarReplacementWithGCBarrier(List list) { Iter iter = list.iter(); - for (;;) { + while (true) { while (iter.next()) {} if (list.head == null) break; list.head = list.head.next; From 8ee384988889d79a60700b5732255269be6c50ab Mon Sep 17 00:00:00 2001 From: MaxXSoft Date: Thu, 22 Aug 2024 16:54:51 +0800 Subject: [PATCH 11/13] Add wrapper method for checking `DomResult` of `all_controls_dominate`. --- src/hotspot/share/opto/memnode.cpp | 31 ++++++++++++++++++------------ src/hotspot/share/opto/memnode.hpp | 8 ++++++-- 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/src/hotspot/share/opto/memnode.cpp b/src/hotspot/share/opto/memnode.cpp index 9e9b2f7205342..baef58987b56f 100644 --- a/src/hotspot/share/opto/memnode.cpp +++ b/src/hotspot/share/opto/memnode.cpp @@ -428,7 +428,12 @@ Node *MemNode::Ideal_common(PhaseGVN *phase, bool can_reshape) { // Used by MemNode::find_previous_store to prove that the // control input of a memory operation predates (dominates) // an allocation it wants to look past. -Node::DomResult MemNode::all_controls_dominate(Node* dom, Node* sub) { +// Returns 'DomResult::Dominate' if all control inputs of 'dom' +// dominate 'sub', 'DomResult::NotDominate' if not, +// and 'DomResult::EncounteredDeadCode' if we can't decide due to +// dead code, but at the end of IGVN, we know the definite result +// once the dead code is cleaned up. +Node::DomResult MemNode::maybe_all_controls_dominate(Node* dom, Node* sub) { if (dom == nullptr || dom->is_top() || sub == nullptr || sub->is_top()) { return DomResult::EncounteredDeadCode; // Conservative answer for dead code } @@ -549,9 +554,9 @@ bool MemNode::detect_ptr_independence(Node* p1, AllocateNode* a1, return (a1 != a2); } else if (a1 != nullptr) { // one allocation a1 // (Note: p2->is_Con implies p2->in(0)->is_Root, which dominates.) - return all_controls_dominate(p2, a1) == DomResult::Dominate; + return all_controls_dominate(p2, a1); } else { //(a2 != null) // one allocation a2 - return all_controls_dominate(p1, a2) == DomResult::Dominate; + return all_controls_dominate(p1, a2); } return false; } @@ -748,7 +753,7 @@ Node* MemNode::find_previous_store(PhaseValues* phase) { known_identical = true; } else if (alloc != nullptr) { known_independent = true; - } else if (all_controls_dominate(this, st_alloc) == DomResult::Dominate) { + } else if (all_controls_dominate(this, st_alloc)) { known_independent = true; } @@ -1581,11 +1586,11 @@ bool LoadNode::can_split_through_phi_base(PhaseGVN* phase) { } if (!mem->is_Phi()) { - if (MemNode::all_controls_dominate(mem, base->in(0)) != DomResult::Dominate) { + if (!MemNode::all_controls_dominate(mem, base->in(0))) { return false; } } else if (base->in(0) != mem->in(0)) { - if (MemNode::all_controls_dominate(mem, base->in(0)) != DomResult::Dominate) { + if (!MemNode::all_controls_dominate(mem, base->in(0))) { return false; } } @@ -1680,20 +1685,20 @@ Node* LoadNode::split_through_phi(PhaseGVN* phase, bool ignore_missing_instance_ region = mem->in(0); // Skip if the region dominates some control edge of the address. // We will check `dom_result` later. - dom_result = MemNode::all_controls_dominate(address, region); + dom_result = MemNode::maybe_all_controls_dominate(address, region); } else if (!mem->is_Phi()) { assert(base_is_phi, "sanity"); region = base->in(0); // Skip if the region dominates some control edge of the memory. // We will check `dom_result` later. - dom_result = MemNode::all_controls_dominate(mem, region); + dom_result = MemNode::maybe_all_controls_dominate(mem, region); } else if (base->in(0) != mem->in(0)) { assert(base_is_phi && mem->is_Phi(), "sanity"); - dom_result = MemNode::all_controls_dominate(mem, base->in(0)); + dom_result = MemNode::maybe_all_controls_dominate(mem, base->in(0)); if (dom_result == DomResult::Dominate) { region = base->in(0); } else { - dom_result = MemNode::all_controls_dominate(address, mem->in(0)); + dom_result = MemNode::maybe_all_controls_dominate(address, mem->in(0)); if (dom_result == DomResult::Dominate) { region = mem->in(0); } @@ -1708,6 +1713,8 @@ Node* LoadNode::split_through_phi(PhaseGVN* phase, bool ignore_missing_instance_ if (dom_result != DomResult::Dominate) { if (dom_result == DomResult::EncounteredDeadCode) { // Wait for the dead code to be removed. + // The dead code will eventually be removed in IGVN, + // so we have an unambiguous result whether it's dominated or not. igvn->_worklist.push(this); } return nullptr; @@ -1860,7 +1867,7 @@ Node *LoadNode::Ideal(PhaseGVN *phase, bool can_reshape) { if (in(MemNode::Control) != nullptr && can_remove_control() && phase->type(base)->higher_equal(TypePtr::NOTNULL) - && all_controls_dominate(base, phase->C->start()) == DomResult::Dominate) { + && all_controls_dominate(base, phase->C->start())) { // A method-invariant, non-null address (constant or 'this' argument). set_req(MemNode::Control, nullptr); progress = true; @@ -4594,7 +4601,7 @@ bool InitializeNode::detect_init_independence(Node* value, PhaseGVN* phase) { // must have preceded the init, or else be equal to the init. // Even after loop optimizations (which might change control edges) // a store is never pinned *before* the availability of its inputs. - if (MemNode::all_controls_dominate(n, this) != DomResult::Dominate) { + if (!MemNode::all_controls_dominate(n, this)) { return false; // failed to prove a good control } } diff --git a/src/hotspot/share/opto/memnode.hpp b/src/hotspot/share/opto/memnode.hpp index e3455a81402e4..7ccb9e4af92b4 100644 --- a/src/hotspot/share/opto/memnode.hpp +++ b/src/hotspot/share/opto/memnode.hpp @@ -106,8 +106,12 @@ class MemNode : public Node { static Node *optimize_simple_memory_chain(Node *mchain, const TypeOopPtr *t_oop, Node *load, PhaseGVN *phase); static Node *optimize_memory_chain(Node *mchain, const TypePtr *t_adr, Node *load, PhaseGVN *phase); - // This one should probably be a phase-specific function: - static DomResult all_controls_dominate(Node* dom, Node* sub); + // The following two should probably be phase-specific functions: + static DomResult maybe_all_controls_dominate(Node* dom, Node* sub); + static bool all_controls_dominate(Node* dom, Node* sub) { + DomResult dom_result = maybe_all_controls_dominate(dom, sub); + return dom_result == DomResult::Dominate; + } virtual const class TypePtr *adr_type() const; // returns bottom_type of address From b5e579fcde87bf7bde8d27521b1fab574358b5af Mon Sep 17 00:00:00 2001 From: MaxXing Date: Thu, 22 Aug 2024 17:34:01 +0800 Subject: [PATCH 12/13] Update comments in method `split_through_phi`. Co-authored-by: Christian Hagedorn --- src/hotspot/share/opto/memnode.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/hotspot/share/opto/memnode.cpp b/src/hotspot/share/opto/memnode.cpp index baef58987b56f..dfbed6eecf284 100644 --- a/src/hotspot/share/opto/memnode.cpp +++ b/src/hotspot/share/opto/memnode.cpp @@ -1712,9 +1712,8 @@ Node* LoadNode::split_through_phi(PhaseGVN* phase, bool ignore_missing_instance_ PhaseIterGVN* igvn = phase->is_IterGVN(); if (dom_result != DomResult::Dominate) { if (dom_result == DomResult::EncounteredDeadCode) { - // Wait for the dead code to be removed. - // The dead code will eventually be removed in IGVN, - // so we have an unambiguous result whether it's dominated or not. + // There is some dead code which eventually will be removed in IGVN. Once this is the case, we get an unambiguous + // dominance result. Push the node to the worklist again until the dead code is removed. igvn->_worklist.push(this); } return nullptr; From f8118454e4dcd665fe2c2337a7044e3cbaf4ad7e Mon Sep 17 00:00:00 2001 From: MaxXSoft Date: Thu, 22 Aug 2024 17:35:58 +0800 Subject: [PATCH 13/13] Fix whitespace. --- src/hotspot/share/opto/memnode.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/hotspot/share/opto/memnode.cpp b/src/hotspot/share/opto/memnode.cpp index dfbed6eecf284..6304567d0b8fd 100644 --- a/src/hotspot/share/opto/memnode.cpp +++ b/src/hotspot/share/opto/memnode.cpp @@ -1712,8 +1712,9 @@ Node* LoadNode::split_through_phi(PhaseGVN* phase, bool ignore_missing_instance_ PhaseIterGVN* igvn = phase->is_IterGVN(); if (dom_result != DomResult::Dominate) { if (dom_result == DomResult::EncounteredDeadCode) { - // There is some dead code which eventually will be removed in IGVN. Once this is the case, we get an unambiguous - // dominance result. Push the node to the worklist again until the dead code is removed. + // There is some dead code which eventually will be removed in IGVN. + // Once this is the case, we get an unambiguous dominance result. + // Push the node to the worklist again until the dead code is removed. igvn->_worklist.push(this); } return nullptr;