285 lines
10 KiB
Diff
285 lines
10 KiB
Diff
|
|
From 8d1f47f09c817f343a5ce5a181d2789f7e1c7c84 Mon Sep 17 00:00:00 2001
|
||
|
|
From: zhangyipeng <zhangyipeng7@huawei.com>
|
||
|
|
Date: Wed, 28 Jul 2021 17:55:27 +0800
|
||
|
|
Subject: [PATCH] [Backport]8069191:moving predicate out of loops
|
||
|
|
may cause array accesses to bypass null check
|
||
|
|
|
||
|
|
Offering: Cloud Compiler JDK
|
||
|
|
Reference: https://bugs.openjdk.java.net/browse/JDK-8069191
|
||
|
|
|
||
|
|
|
||
|
|
Signed-off-by: Wang Kun <wangkun49@huawei.com>
|
||
|
|
---
|
||
|
|
diff --git a/hotspot/src/share/vm/opto/compile.cpp b/hotspot/src/share/vm/opto/compile.cpp
|
||
|
|
index 5bee20b..832033a 100644
|
||
|
|
--- a/hotspot/src/share/vm/opto/compile.cpp
|
||
|
|
+++ b/hotspot/src/share/vm/opto/compile.cpp
|
||
|
|
@@ -2886,9 +2886,38 @@ void Compile::final_graph_reshaping_impl( Node *n, Final_Reshape_Counts &frc) {
|
||
|
|
break;
|
||
|
|
}
|
||
|
|
|
||
|
|
-#ifdef _LP64
|
||
|
|
- case Op_CastPP:
|
||
|
|
- if (n->in(1)->is_DecodeN() && Matcher::gen_narrow_oop_implicit_null_checks()) {
|
||
|
|
+ case Op_CastPP: {
|
||
|
|
+ // Remove CastPP nodes to gain more freedom during scheduling but
|
||
|
|
+ // keep the dependency they encode as control or precedence edges
|
||
|
|
+ // (if control is set already) on memory operations. Some CastPP
|
||
|
|
+ // nodes don't have a control (don't carry a dependency): skip
|
||
|
|
+ // those.
|
||
|
|
+ if (n->in(0) != NULL) {
|
||
|
|
+ ResourceMark rm;
|
||
|
|
+ Unique_Node_List wq;
|
||
|
|
+ wq.push(n);
|
||
|
|
+ for (uint next = 0; next < wq.size(); ++next) {
|
||
|
|
+ Node *m = wq.at(next);
|
||
|
|
+ for (DUIterator_Fast imax, i = m->fast_outs(imax); i < imax; i++) {
|
||
|
|
+ Node* use = m->fast_out(i);
|
||
|
|
+ if (use->is_Mem() || use->is_EncodeNarrowPtr()) {
|
||
|
|
+ use->ensure_control_or_add_prec(n->in(0));
|
||
|
|
+ } else if (use->in(0) == NULL) {
|
||
|
|
+ switch(use->Opcode()) {
|
||
|
|
+ case Op_AddP:
|
||
|
|
+ case Op_DecodeN:
|
||
|
|
+ case Op_DecodeNKlass:
|
||
|
|
+ case Op_CheckCastPP:
|
||
|
|
+ case Op_CastPP:
|
||
|
|
+ wq.push(use);
|
||
|
|
+ break;
|
||
|
|
+ }
|
||
|
|
+ }
|
||
|
|
+ }
|
||
|
|
+ }
|
||
|
|
+ }
|
||
|
|
+ const bool is_LP64 = LP64_ONLY(true) NOT_LP64(false);
|
||
|
|
+ if (is_LP64 && n->in(1)->is_DecodeN() && Matcher::gen_narrow_oop_implicit_null_checks()) {
|
||
|
|
Node* in1 = n->in(1);
|
||
|
|
const Type* t = n->bottom_type();
|
||
|
|
Node* new_in1 = in1->clone();
|
||
|
|
@@ -2921,9 +2950,15 @@ void Compile::final_graph_reshaping_impl( Node *n, Final_Reshape_Counts &frc) {
|
||
|
|
if (in1->outcnt() == 0) {
|
||
|
|
in1->disconnect_inputs(NULL, this);
|
||
|
|
}
|
||
|
|
+ } else {
|
||
|
|
+ n->subsume_by(n->in(1), this);
|
||
|
|
+ if (n->outcnt() == 0) {
|
||
|
|
+ n->disconnect_inputs(NULL, this);
|
||
|
|
+ }
|
||
|
|
}
|
||
|
|
break;
|
||
|
|
-
|
||
|
|
+ }
|
||
|
|
+#ifdef _LP64
|
||
|
|
case Op_CmpP:
|
||
|
|
// Do this transformation here to preserve CmpPNode::sub() and
|
||
|
|
// other TypePtr related Ideal optimizations (for example, ptr nullness).
|
||
|
|
diff --git a/hotspot/src/share/vm/opto/gcm.cpp b/hotspot/src/share/vm/opto/gcm.cpp
|
||
|
|
index f51484e..893becc 100644
|
||
|
|
--- a/hotspot/src/share/vm/opto/gcm.cpp
|
||
|
|
+++ b/hotspot/src/share/vm/opto/gcm.cpp
|
||
|
|
@@ -116,6 +116,9 @@ void PhaseCFG::replace_block_proj_ctrl( Node *n ) {
|
||
|
|
}
|
||
|
|
}
|
||
|
|
|
||
|
|
+static bool is_dominator(Block* d, Block* n) {
|
||
|
|
+ return d->dom_lca(n) == d;
|
||
|
|
+}
|
||
|
|
|
||
|
|
//------------------------------schedule_pinned_nodes--------------------------
|
||
|
|
// Set the basic block for Nodes pinned into blocks
|
||
|
|
@@ -138,6 +141,42 @@ void PhaseCFG::schedule_pinned_nodes(VectorSet &visited) {
|
||
|
|
schedule_node_into_block(node, block);
|
||
|
|
}
|
||
|
|
|
||
|
|
+ // If the node has precedence edges (added when CastPP nodes are
|
||
|
|
+ // removed in final_graph_reshaping), fix the control of the
|
||
|
|
+ // node to cover the precedence edges and remove the
|
||
|
|
+ // dependencies.
|
||
|
|
+ Node* n = NULL;
|
||
|
|
+ for (uint i = node->len()-1; i >= node->req(); i--) {
|
||
|
|
+ Node* m = node->in(i);
|
||
|
|
+ if (m == NULL) continue;
|
||
|
|
+ // Skip the precedence edge if the test that guarded a CastPP:
|
||
|
|
+ // - was optimized out during escape analysis
|
||
|
|
+ // (OptimizePtrCompare): the CastPP's control isn't an end of
|
||
|
|
+ // block.
|
||
|
|
+ // - is moved in the branch of a dominating If: the control of
|
||
|
|
+ // the CastPP is then a Region.
|
||
|
|
+ if (m->is_block_proj() || m->is_block_start()) {
|
||
|
|
+ node->rm_prec(i);
|
||
|
|
+ if (n == NULL) {
|
||
|
|
+ n = m;
|
||
|
|
+ } else {
|
||
|
|
+ Block* bn = get_block_for_node(n);
|
||
|
|
+ Block* bm = get_block_for_node(m);
|
||
|
|
+ assert(is_dominator(bn, bm) || is_dominator(bm, bn), "one must dominate the other");
|
||
|
|
+ n = is_dominator(bn, bm) ? m : n;
|
||
|
|
+ }
|
||
|
|
+ }
|
||
|
|
+ }
|
||
|
|
+ if (n != NULL) {
|
||
|
|
+ assert(node->in(0), "control should have been set");
|
||
|
|
+ Block* bn = get_block_for_node(n);
|
||
|
|
+ Block* bnode = get_block_for_node(node->in(0));
|
||
|
|
+ assert(is_dominator(bn, bnode) || is_dominator(bnode, bn), "one must dominate the other");
|
||
|
|
+ if (!is_dominator(bn, bnode)) {
|
||
|
|
+ node->set_req(0, n);
|
||
|
|
+ }
|
||
|
|
+ }
|
||
|
|
+
|
||
|
|
// process all inputs that are non NULL
|
||
|
|
for (int i = node->req() - 1; i >= 0; --i) {
|
||
|
|
if (node->in(i) != NULL) {
|
||
|
|
diff --git a/hotspot/src/share/vm/opto/matcher.cpp b/hotspot/src/share/vm/opto/matcher.cpp
|
||
|
|
index 0c9a8d4..1036df2 100644
|
||
|
|
--- a/hotspot/src/share/vm/opto/matcher.cpp
|
||
|
|
+++ b/hotspot/src/share/vm/opto/matcher.cpp
|
||
|
|
@@ -1068,6 +1068,15 @@ Node *Matcher::xform( Node *n, int max_stack ) {
|
||
|
|
mstack.push(m, Visit, n, -1);
|
||
|
|
}
|
||
|
|
|
||
|
|
+ // Handle precedence edges for interior nodes
|
||
|
|
+ for (i = n->len()-1; (uint)i >= n->req(); i--) {
|
||
|
|
+ Node *m = n->in(i);
|
||
|
|
+ if (m == NULL || C->node_arena()->contains(m)) continue;
|
||
|
|
+ n->rm_prec(i);
|
||
|
|
+ // set -1 to call add_prec() instead of set_req() during Step1
|
||
|
|
+ mstack.push(m, Visit, n, -1);
|
||
|
|
+ }
|
||
|
|
+
|
||
|
|
// For constant debug info, I'd rather have unmatched constants.
|
||
|
|
int cnt = n->req();
|
||
|
|
JVMState* jvms = n->jvms();
|
||
|
|
@@ -1758,6 +1767,14 @@ MachNode *Matcher::ReduceInst( State *s, int rule, Node *&mem ) {
|
||
|
|
return ex;
|
||
|
|
}
|
||
|
|
|
||
|
|
+void Matcher::handle_precedence_edges(Node* n, MachNode *mach) {
|
||
|
|
+ for (uint i = n->req(); i < n->len(); i++) {
|
||
|
|
+ if (n->in(i) != NULL) {
|
||
|
|
+ mach->add_prec(n->in(i));
|
||
|
|
+ }
|
||
|
|
+ }
|
||
|
|
+}
|
||
|
|
+
|
||
|
|
void Matcher::ReduceInst_Chain_Rule( State *s, int rule, Node *&mem, MachNode *mach ) {
|
||
|
|
// 'op' is what I am expecting to receive
|
||
|
|
int op = _leftOp[rule];
|
||
|
|
@@ -1792,6 +1809,8 @@ void Matcher::ReduceInst_Chain_Rule( State *s, int rule, Node *&mem, MachNode *m
|
||
|
|
|
||
|
|
|
||
|
|
uint Matcher::ReduceInst_Interior( State *s, int rule, Node *&mem, MachNode *mach, uint num_opnds ) {
|
||
|
|
+ handle_precedence_edges(s->_leaf, mach);
|
||
|
|
+
|
||
|
|
if( s->_leaf->is_Load() ) {
|
||
|
|
Node *mem2 = s->_leaf->in(MemNode::Memory);
|
||
|
|
assert( mem == (Node*)1 || mem == mem2, "multiple Memories being matched at once?" );
|
||
|
|
@@ -1874,6 +1893,9 @@ void Matcher::ReduceOper( State *s, int rule, Node *&mem, MachNode *mach ) {
|
||
|
|
mem = s->_leaf->in(MemNode::Memory);
|
||
|
|
debug_only(_mem_node = s->_leaf;)
|
||
|
|
}
|
||
|
|
+
|
||
|
|
+ handle_precedence_edges(s->_leaf, mach);
|
||
|
|
+
|
||
|
|
if( s->_leaf->in(0) && s->_leaf->req() > 1) {
|
||
|
|
if( !mach->in(0) )
|
||
|
|
mach->set_req(0,s->_leaf->in(0));
|
||
|
|
diff --git a/hotspot/src/share/vm/opto/matcher.hpp b/hotspot/src/share/vm/opto/matcher.hpp
|
||
|
|
index 2f2dc5b..f882ad2 100644
|
||
|
|
--- a/hotspot/src/share/vm/opto/matcher.hpp
|
||
|
|
+++ b/hotspot/src/share/vm/opto/matcher.hpp
|
||
|
|
@@ -124,6 +124,8 @@ class Matcher : public PhaseTransform {
|
||
|
|
// Mach node for ConP #NULL
|
||
|
|
MachNode* _mach_null;
|
||
|
|
|
||
|
|
+ void handle_precedence_edges(Node* n, MachNode *mach);
|
||
|
|
+
|
||
|
|
public:
|
||
|
|
int LabelRootDepth;
|
||
|
|
// Convert ideal machine register to a register mask for spill-loads
|
||
|
|
diff --git a/hotspot/src/share/vm/opto/node.cpp b/hotspot/src/share/vm/opto/node.cpp
|
||
|
|
index d6b9d15..7ea783e 100644
|
||
|
|
--- a/hotspot/src/share/vm/opto/node.cpp
|
||
|
|
+++ b/hotspot/src/share/vm/opto/node.cpp
|
||
|
|
@@ -1432,12 +1432,6 @@ bool Node::remove_dead_region(PhaseGVN *phase, bool can_reshape) {
|
||
|
|
return false;
|
||
|
|
}
|
||
|
|
|
||
|
|
-//------------------------------Ideal_DU_postCCP-------------------------------
|
||
|
|
-// Idealize graph, using DU info. Must clone result into new-space
|
||
|
|
-Node *Node::Ideal_DU_postCCP( PhaseCCP * ) {
|
||
|
|
- return NULL; // Default to no change
|
||
|
|
-}
|
||
|
|
-
|
||
|
|
//------------------------------hash-------------------------------------------
|
||
|
|
// Hash function over Nodes.
|
||
|
|
uint Node::hash() const {
|
||
|
|
@@ -2126,6 +2120,14 @@ Node* Node::unique_ctrl_out() {
|
||
|
|
return found;
|
||
|
|
}
|
||
|
|
|
||
|
|
+void Node::ensure_control_or_add_prec(Node* c) {
|
||
|
|
+ if (in(0) == NULL) {
|
||
|
|
+ set_req(0, c);
|
||
|
|
+ } else if (in(0) != c) {
|
||
|
|
+ add_prec(c);
|
||
|
|
+ }
|
||
|
|
+}
|
||
|
|
+
|
||
|
|
//=============================================================================
|
||
|
|
//------------------------------yank-------------------------------------------
|
||
|
|
// Find and remove
|
||
|
|
diff --git a/hotspot/src/share/vm/opto/node.hpp b/hotspot/src/share/vm/opto/node.hpp
|
||
|
|
index 270b966..f0a6ee0 100644
|
||
|
|
--- a/hotspot/src/share/vm/opto/node.hpp
|
||
|
|
+++ b/hotspot/src/share/vm/opto/node.hpp
|
||
|
|
@@ -926,9 +926,6 @@ protected:
|
||
|
|
bool remove_dead_region(PhaseGVN *phase, bool can_reshape);
|
||
|
|
public:
|
||
|
|
|
||
|
|
- // Idealize graph, using DU info. Done after constant propagation
|
||
|
|
- virtual Node *Ideal_DU_postCCP( PhaseCCP *ccp );
|
||
|
|
-
|
||
|
|
// See if there is valid pipeline info
|
||
|
|
static const Pipeline *pipeline_class();
|
||
|
|
virtual const Pipeline *pipeline() const;
|
||
|
|
@@ -962,6 +959,9 @@ public:
|
||
|
|
// Return the unique control out if only one. Null if none or more than one.
|
||
|
|
Node* unique_ctrl_out();
|
||
|
|
|
||
|
|
+ // Set control or add control as precedence edge
|
||
|
|
+ void ensure_control_or_add_prec(Node* c);
|
||
|
|
+
|
||
|
|
//----------------- Code Generation
|
||
|
|
|
||
|
|
// Ideal register class for Matching. Zero means unmatched instruction
|
||
|
|
diff --git a/hotspot/src/share/vm/opto/phaseX.cpp b/hotspot/src/share/vm/opto/phaseX.cpp
|
||
|
|
index 9c4a705..ae1031d 100644
|
||
|
|
--- a/hotspot/src/share/vm/opto/phaseX.cpp
|
||
|
|
+++ b/hotspot/src/share/vm/opto/phaseX.cpp
|
||
|
|
@@ -1774,11 +1774,6 @@ Node *PhaseCCP::transform_once( Node *n ) {
|
||
|
|
_worklist.push(n); // n re-enters the hash table via the worklist
|
||
|
|
}
|
||
|
|
|
||
|
|
- // Idealize graph using DU info. Must clone() into new-space.
|
||
|
|
- // DU info is generally used to show profitability, progress or safety
|
||
|
|
- // (but generally not needed for correctness).
|
||
|
|
- Node *nn = n->Ideal_DU_postCCP(this);
|
||
|
|
-
|
||
|
|
// TEMPORARY fix to ensure that 2nd GVN pass eliminates NULL checks
|
||
|
|
switch( n->Opcode() ) {
|
||
|
|
case Op_FastLock: // Revisit FastLocks for lock coarsening
|
||
|
|
@@ -1795,12 +1790,6 @@ Node *PhaseCCP::transform_once( Node *n ) {
|
||
|
|
default:
|
||
|
|
break;
|
||
|
|
}
|
||
|
|
- if( nn ) {
|
||
|
|
- _worklist.push(n);
|
||
|
|
- // Put users of 'n' onto worklist for second igvn transform
|
||
|
|
- add_users_to_worklist(n);
|
||
|
|
- return nn;
|
||
|
|
- }
|
||
|
|
|
||
|
|
return n;
|
||
|
|
}
|