8294540: Remove Opaque2Node: it is broken and triggers assert

Reviewed-by: chagedorn, kvn
This commit is contained in:
Emanuel Peter 2022-12-05 08:30:31 +00:00
parent 82561de722
commit 619b68c5d1
11 changed files with 16 additions and 187 deletions

@ -268,7 +268,6 @@ macro(OnSpinWait)
macro(Opaque1)
macro(OpaqueLoopInit)
macro(OpaqueLoopStride)
macro(Opaque2)
macro(Opaque3)
macro(Opaque4)
macro(ProfileBoolean)

@ -3161,7 +3161,6 @@ void Compile::final_graph_reshaping_main_switch(Node* n, Final_Reshape_Counts& f
frc.inc_double_count();
break;
case Op_Opaque1: // Remove Opaque Nodes before matching
case Op_Opaque2: // Remove Opaque Nodes before matching
case Op_Opaque3:
n->subsume_by(n->in(1), this);
break;

@ -2287,18 +2287,7 @@ void PhaseIdealLoop::do_unroll(IdealLoopTree *loop, Node_List &old_new, bool adj
set_ctrl(new_limit, C->root());
} else {
// Limit is not constant.
if (loop_head->unrolled_count() == 1) { // only for first unroll
// Separate limit by Opaque node in case it is an incremented
// variable from previous loop to avoid using pre-incremented
// value which could increase register pressure.
// Otherwise reorg_offsets() optimization will create a separate
// Opaque node for each use of trip-counter and as result
// zero trip guard limit will be different from loop limit.
assert(has_ctrl(opaq), "should have it");
Node* opaq_ctrl = get_ctrl(opaq);
limit = new Opaque2Node(C, limit);
register_new_node(limit, opaq_ctrl);
}
assert(loop_head->unrolled_count() != 1 || has_ctrl(opaq), "should have opaque for first unroll");
if ((stride_con > 0 && (java_subtract(limit_type->_lo, stride_con) < limit_type->_lo)) ||
(stride_con < 0 && (java_subtract(limit_type->_hi, stride_con) > limit_type->_hi))) {
// No underflow.
@ -2346,20 +2335,6 @@ void PhaseIdealLoop::do_unroll(IdealLoopTree *loop, Node_List &old_new, bool adj
new_limit = new CMoveINode(adj_bool, adj_limit, adj_max, TypeInt::INT);
}
register_new_node(new_limit, ctrl);
if (loop_head->unrolled_count() == 1) {
// The Opaque2 node created above (in the case of the first unrolling) hides the type of the loop limit.
// As a result, if the iv Phi constant folds (because it captured the iteration range), the exit test won't
// constant fold and the graph contains a broken counted loop.
const Type* new_limit_t;
if (stride_con > 0) {
new_limit_t = TypeInt::make(min_jint, limit_type->_hi, limit_type->_widen);
} else {
assert(stride_con < 0, "stride can't be 0");
new_limit_t = TypeInt::make(limit_type->_lo, max_jint, limit_type->_widen);
}
new_limit = new CastIINode(new_limit, new_limit_t);
register_new_node(new_limit, ctrl);
}
}
assert(new_limit != NULL, "");
@ -3940,10 +3915,6 @@ bool IdealLoopTree::iteration_split(PhaseIdealLoop* phase, Node_List &old_new) {
}
}
// Minor offset re-organization to remove loop-fallout uses of
// trip counter when there was no major reshaping.
phase->reorg_offsets(this);
if (_next && !_next->iteration_split(phase, old_new)) {
return false;
}

@ -1522,12 +1522,6 @@ public:
// Attempt to use a conditional move instead of a phi/branch
Node *conditional_move( Node *n );
// Reorganize offset computations to lower register pressure.
// Mostly prevent loop-fallout uses of the pre-incremented trip counter
// (which are then alive with the post-incremented trip counter
// forcing an extra register move)
void reorg_offsets( IdealLoopTree *loop );
// Check for aggressive application of 'split-if' optimization,
// using basic block level info.
void split_if_with_blocks ( VectorSet &visited, Node_Stack &nstack);

@ -1023,8 +1023,7 @@ Node *PhaseIdealLoop::split_if_with_blocks_pre( Node *n ) {
if (n->is_CFG() || n->is_LoadStore()) {
return n;
}
if (n->is_Opaque1() || // Opaque nodes cannot be mod'd
n_op == Op_Opaque2) {
if (n->is_Opaque1()) { // Opaque nodes cannot be mod'd
if (!C->major_progress()) { // If chance of no more loop opts...
_igvn._worklist.push(n); // maybe we'll remove them
}
@ -1426,14 +1425,6 @@ void PhaseIdealLoop::split_if_with_blocks_post(Node *n) {
try_sink_out_of_loop(n);
try_move_store_after_loop(n);
// Check for Opaque2's who's loop has disappeared - who's input is in the
// same loop nest as their output. Remove 'em, they are no longer useful.
if( n_op == Op_Opaque2 &&
n->in(1) != NULL &&
get_loop(get_ctrl(n)) == get_loop(get_ctrl(n->in(1))) ) {
_igvn.replace_node( n, n->in(1) );
}
}
// Transform:
@ -4106,89 +4097,3 @@ bool PhaseIdealLoop::duplicate_loop_backedge(IdealLoopTree *loop, Node_List &old
return true;
}
//------------------------------reorg_offsets----------------------------------
// Reorganize offset computations to lower register pressure. Mostly
// prevent loop-fallout uses of the pre-incremented trip counter (which are
// then alive with the post-incremented trip counter forcing an extra
// register move):
//
// iv Phi iv Phi
// | |
// | AddI (+stride)
// | |
// | Opaque2 # Blocks IGVN from folding these nodes until loop opts are over.
// | ====> |
// | AddI (-stride)
// | |
// | CastII # Preserve type of iv Phi
// | |
// Outside Use Outside Use
//
void PhaseIdealLoop::reorg_offsets(IdealLoopTree *loop) {
// Perform it only for canonical counted loops.
// Loop's shape could be messed up by iteration_split_impl.
if (!loop->_head->is_CountedLoop())
return;
if (!loop->_head->as_Loop()->is_valid_counted_loop(T_INT))
return;
CountedLoopNode *cl = loop->_head->as_CountedLoop();
CountedLoopEndNode *cle = cl->loopexit();
Node *exit = cle->proj_out(false);
Node *phi = cl->phi();
// Check for the special case when using the pre-incremented trip-counter on
// the fall-out path (forces the pre-incremented and post-incremented trip
// counter to be live at the same time). Fix this by adjusting to use the
// post-increment trip counter.
bool progress = true;
while (progress) {
progress = false;
for (DUIterator_Fast imax, i = phi->fast_outs(imax); i < imax; i++) {
Node* use = phi->fast_out(i); // User of trip-counter
if (!has_ctrl(use)) continue;
Node *u_ctrl = get_ctrl(use);
if (use->is_Phi()) {
u_ctrl = NULL;
for (uint j = 1; j < use->req(); j++)
if (use->in(j) == phi)
u_ctrl = dom_lca(u_ctrl, use->in(0)->in(j));
}
IdealLoopTree *u_loop = get_loop(u_ctrl);
// Look for loop-invariant use
if (u_loop == loop) continue;
if (loop->is_member(u_loop)) continue;
// Check that use is live out the bottom. Assuming the trip-counter
// update is right at the bottom, uses of of the loop middle are ok.
if (dom_lca(exit, u_ctrl) != exit) continue;
// Hit! Refactor use to use the post-incremented tripcounter.
// Compute a post-increment tripcounter.
Node* c = exit;
if (cl->is_strip_mined()) {
IdealLoopTree* outer_loop = get_loop(cl->outer_loop());
if (!outer_loop->is_member(u_loop)) {
c = cl->outer_loop_exit();
}
}
Node *opaq = new Opaque2Node(C, cle->incr());
register_new_node(opaq, c);
Node *neg_stride = _igvn.intcon(-cle->stride_con());
set_ctrl(neg_stride, C->root());
Node *post = new AddINode(opaq, neg_stride);
register_new_node(post, c);
post = new CastIINode(post, phi->bottom_type()); // preserve the iv phi's type
register_new_node(post, c);
_igvn.rehash_node_delayed(use);
for (uint j = 1; j < use->req(); j++) {
if (use->in(j) == phi)
use->set_req(j, post);
}
// Since DU info changed, rerun loop
progress = true;
break;
}
}
}

@ -2371,7 +2371,6 @@ void PhaseMacroExpand::eliminate_macro_nodes() {
break;
default:
assert(n->Opcode() == Op_LoopLimit ||
n->Opcode() == Op_Opaque2 ||
n->Opcode() == Op_Opaque3 ||
n->Opcode() == Op_Opaque4 ||
BarrierSet::barrier_set()->barrier_set_c2()->is_gc_barrier_node(n),
@ -2414,7 +2413,7 @@ bool PhaseMacroExpand::expand_macro_nodes() {
C->remove_macro_node(n);
_igvn._worklist.push(n);
success = true;
} else if (n->is_Opaque1() || n->Opcode() == Op_Opaque2) {
} else if (n->is_Opaque1()) {
_igvn.replace_node(n, n->in(1));
success = true;
#if INCLUDE_RTM_OPT

@ -44,20 +44,8 @@ Node* Opaque1Node::Identity(PhaseGVN* phase) {
return this;
}
//=============================================================================
// A node to prevent unwanted optimizations. Allows constant folding. Stops
// value-numbering, most Ideal calls or Identity functions. This Node is
// specifically designed to prevent the pre-increment value of a loop trip
// counter from being live out of the bottom of the loop (hence causing the
// pre- and post-increment values both being live and thus requiring an extra
// temp register and an extra move). If we "accidentally" optimize through
// this kind of a Node, we'll get slightly pessimal, but correct, code. Thus
// it's OK to be slightly sloppy on optimizations here.
// Do NOT remove the opaque node until no more loop opts can happen. Opaque1
// and Opaque2 nodes are removed together in order to optimize loops away
// before macro expansion.
Node* Opaque2Node::Identity(PhaseGVN* phase) {
// Do NOT remove the opaque node until no more loop opts can happen.
Node* Opaque3Node::Identity(PhaseGVN* phase) {
if (phase->C->post_loop_opts_phase()) {
return in(1);
} else {
@ -67,8 +55,8 @@ Node* Opaque2Node::Identity(PhaseGVN* phase) {
}
// Do not allow value-numbering
uint Opaque2Node::hash() const { return NO_HASH; }
bool Opaque2Node::cmp( const Node &n ) const {
uint Opaque3Node::hash() const { return NO_HASH; }
bool Opaque3Node::cmp(const Node &n) const {
return (&n == this); // Always fail except on self
}

@ -70,20 +70,16 @@ class OpaqueLoopStrideNode : public Opaque1Node {
virtual int Opcode() const;
};
//------------------------------Opaque2Node------------------------------------
// A node to prevent unwanted optimizations. Allows constant folding. Stops
// value-numbering, most Ideal calls or Identity functions. This Node is
// specifically designed to prevent the pre-increment value of a loop trip
// counter from being live out of the bottom of the loop (hence causing the
// pre- and post-increment values both being live and thus requiring an extra
// temp register and an extra move). If we "accidentally" optimize through
// this kind of a Node, we'll get slightly pessimal, but correct, code. Thus
// it's OK to be slightly sloppy on optimizations here.
class Opaque2Node : public Node {
virtual uint hash() const ; // { return NO_HASH; }
virtual bool cmp( const Node &n ) const;
//------------------------------Opaque3Node------------------------------------
// A node to prevent unwanted optimizations. Will be optimized only during
// macro nodes expansion.
class Opaque3Node : public Node {
int _opt; // what optimization it was used for
virtual uint hash() const;
virtual bool cmp(const Node &n) const;
public:
Opaque2Node( Compile* C, Node *n ) : Node(0,n) {
enum { RTM_OPT };
Opaque3Node(Compile* C, Node* n, int opt) : Node(0, n), _opt(opt) {
// Put it on the Macro nodes list to removed during macro nodes expansion.
init_flags(Flag_is_macro);
C->add_macro_node(this);
@ -91,17 +87,6 @@ class Opaque2Node : public Node {
virtual int Opcode() const;
virtual const Type* bottom_type() const { return TypeInt::INT; }
virtual Node* Identity(PhaseGVN* phase);
};
//------------------------------Opaque3Node------------------------------------
// A node to prevent unwanted optimizations. Will be optimized only during
// macro nodes expansion.
class Opaque3Node : public Opaque2Node {
int _opt; // what optimization it was used for
public:
enum { RTM_OPT };
Opaque3Node(Compile* C, Node *n, int opt) : Opaque2Node(C, n), _opt(opt) {}
virtual int Opcode() const;
bool rtm_opt() const { return (_opt == RTM_OPT); }
};

@ -2116,7 +2116,6 @@ Node *PhaseCCP::transform_once( Node *n ) {
case Op_CountedLoop:
case Op_Conv2B:
case Op_Opaque1:
case Op_Opaque2:
_worklist.push(n);
break;
default:

@ -69,15 +69,6 @@ Node* SubNode::Identity(PhaseGVN* phase) {
if (in(1)->in(1) == in(2)) {
return in(1)->in(2);
}
// Also catch: "(X + Opaque2(Y)) - Y". In this case, 'Y' is a loop-varying
// trip counter and X is likely to be loop-invariant (that's how O2 Nodes
// are originally used, although the optimizer sometimes jiggers things).
// This folding through an O2 removes a loop-exit use of a loop-varying
// value and generally lowers register pressure in and around the loop.
if (in(1)->in(2)->Opcode() == Op_Opaque2 && in(1)->in(2)->in(1) == in(2)) {
return in(1)->in(1);
}
}
return ( phase->type( in(2) )->higher_equal( zero ) ) ? in(1) : this;

@ -1570,7 +1570,6 @@
declare_c2_type(InitializeNode, MemBarNode) \
declare_c2_type(ThreadLocalNode, Node) \
declare_c2_type(Opaque1Node, Node) \
declare_c2_type(Opaque2Node, Node) \
declare_c2_type(PartialSubtypeCheckNode, Node) \
declare_c2_type(MoveI2FNode, Node) \
declare_c2_type(MoveL2DNode, Node) \