Skip to content

Commit

Permalink
8303466: C2: failed: malformed control flow. Limit type made precise …
Browse files Browse the repository at this point in the history
…with MaxL/MinL

Reviewed-by: andrew, roland
Backport-of: cc894d849aa5f730d5a806acfc7a237cf5170af1
  • Loading branch information
martinuy committed Jul 1, 2024
1 parent 385e34d commit 54f7734
Show file tree
Hide file tree
Showing 11 changed files with 343 additions and 63 deletions.
134 changes: 134 additions & 0 deletions hotspot/src/share/vm/opto/addnode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1024,6 +1024,140 @@ Node *MinINode::Ideal(PhaseGVN *phase, bool can_reshape) {
return NULL;
}

// Collapse the "addition with overflow-protection" pattern, and the symmetrical
// "subtraction with underflow-protection" pattern. These are created during the
// unrolling, when we have to adjust the limit by subtracting the stride, but want
// to protect against underflow: MaxL(SubL(limit, stride), min_jint).
// If we have more than one of those in a sequence:
//
// x con2
// | |
// AddL clamp2
// | |
// Max/MinL con1
// | |
// AddL clamp1
// | |
// Max/MinL (n)
//
// We want to collapse it to:
//
// x con1 con2
// | | |
// | AddLNode (new_con)
// | |
// AddLNode clamp1
// | |
// Max/MinL (n)
//
// Note: we assume that SubL was already replaced by an AddL, and that the stride
// has its sign flipped: SubL(limit, stride) -> AddL(limit, -stride).
static bool is_clamp(PhaseGVN* phase, Node* n, Node* c) {
// Check that the two clamps have the correct values.
jlong clamp = (n->Opcode() == Op_MaxL) ? min_jint : max_jint;
const TypeLong* t = phase->type(c)->isa_long();
return t != NULL && t->is_con() &&
t->get_con() == clamp;
}

static bool is_sub_con(PhaseGVN* phase, Node* n, Node* c) {
// Check that the constants are negative if MaxL, and positive if MinL.
const TypeLong* t = phase->type(c)->isa_long();
return t != NULL && t->is_con() &&
t->get_con() < max_jint && t->get_con() > min_jint &&
(t->get_con() < 0) == (n->Opcode() == Op_MaxL);
}

Node* fold_subI_no_underflow_pattern(Node* n, PhaseGVN* phase) {
assert(n->Opcode() == Op_MaxL || n->Opcode() == Op_MinL, "sanity");
// Verify the graph level by level:
Node* add1 = n->in(1);
Node* clamp1 = n->in(2);
if (add1->Opcode() == Op_AddL && is_clamp(phase, n, clamp1)) {
Node* max2 = add1->in(1);
Node* con1 = add1->in(2);
if (max2->Opcode() == n->Opcode() && is_sub_con(phase, n, con1)) {
Node* add2 = max2->in(1);
Node* clamp2 = max2->in(2);
if (add2->Opcode() == Op_AddL && is_clamp(phase, n, clamp2)) {
Node* x = add2->in(1);
Node* con2 = add2->in(2);
if (is_sub_con(phase, n, con2)) {
Node* new_con = phase->transform(new (phase->C) AddLNode(con1, con2));
Node* new_sub = phase->transform(new (phase->C) AddLNode(x, new_con));
n->set_req_X(1, new_sub, phase);
return n;
}
}
}
}
return NULL;
}

const Type* MaxLNode::add_ring(const Type* t0, const Type* t1) const {
const TypeLong* r0 = t0->is_long();
const TypeLong* r1 = t1->is_long();

return TypeLong::make(MAX2(r0->_lo, r1->_lo), MAX2(r0->_hi, r1->_hi), MAX2(r0->_widen, r1->_widen));
}

Node* MaxLNode::Identity(PhaseGVN* phase) {
const TypeLong* t1 = phase->type(in(1))->is_long();
const TypeLong* t2 = phase->type(in(2))->is_long();

// Can we determine maximum statically?
if (t1->_lo >= t2->_hi) {
return in(1);
} else if (t2->_lo >= t1->_hi) {
return in(2);
}

return MaxNode::Identity(phase);
}

Node* MaxLNode::Ideal(PhaseGVN* phase, bool can_reshape) {
Node* n = AddNode::Ideal(phase, can_reshape);
if (n != NULL) {
return n;
}
if (can_reshape) {
return fold_subI_no_underflow_pattern(this, phase);
}
return NULL;
}

const Type* MinLNode::add_ring(const Type* t0, const Type* t1) const {
const TypeLong* r0 = t0->is_long();
const TypeLong* r1 = t1->is_long();

return TypeLong::make(MIN2(r0->_lo, r1->_lo), MIN2(r0->_hi, r1->_hi), MIN2(r0->_widen, r1->_widen));
}

Node* MinLNode::Identity(PhaseGVN* phase) {
const TypeLong* t1 = phase->type(in(1))->is_long();
const TypeLong* t2 = phase->type(in(2))->is_long();

// Can we determine minimum statically?
if (t1->_lo >= t2->_hi) {
return in(2);
} else if (t2->_lo >= t1->_hi) {
return in(1);
}

return MaxNode::Identity(phase);
}

Node* MinLNode::Ideal(PhaseGVN* phase, bool can_reshape) {
Node* n = AddNode::Ideal(phase, can_reshape);
if (n != NULL) {
return n;
}
if (can_reshape) {
return fold_subI_no_underflow_pattern(this, phase);
}
return NULL;
}

//------------------------------add_ring---------------------------------------
// Supplied function returns the sum of the inputs.
const Type *MinINode::add_ring( const Type *t0, const Type *t1 ) const {
Expand Down
34 changes: 34 additions & 0 deletions hotspot/src/share/vm/opto/addnode.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -279,4 +279,38 @@ class MinINode : public MaxNode {
virtual Node *Ideal(PhaseGVN *phase, bool can_reshape);
};

//------------------------------MaxLNode---------------------------------------
// MAXimum of 2 longs.
class MaxLNode : public MaxNode {
public:
MaxLNode(Compile* C, Node* in1, Node* in2) : MaxNode(in1, in2) {
init_flags(Flag_is_macro);
C->add_macro_node(this);
}
virtual int Opcode() const;
virtual const Type* add_ring(const Type* t0, const Type* t1) const;
virtual const Type* add_id() const { return TypeLong::make(min_jlong); }
virtual const Type* bottom_type() const { return TypeLong::LONG; }
virtual uint ideal_reg() const { return Op_RegL; }
virtual Node* Identity(PhaseGVN* phase);
virtual Node* Ideal(PhaseGVN *phase, bool can_reshape);
};

//------------------------------MinLNode---------------------------------------
// MINimum of 2 longs.
class MinLNode : public MaxNode {
public:
MinLNode(Compile* C, Node* in1, Node* in2) : MaxNode(in1, in2) {
init_flags(Flag_is_macro);
C->add_macro_node(this);
}
virtual int Opcode() const;
virtual const Type* add_ring(const Type* t0, const Type* t1) const;
virtual const Type* add_id() const { return TypeLong::make(max_jlong); }
virtual const Type* bottom_type() const { return TypeLong::LONG; }
virtual uint ideal_reg() const { return Op_RegL; }
virtual Node* Identity(PhaseGVN* phase);
virtual Node* Ideal(PhaseGVN* phase, bool can_reshape);
};

#endif // SHARE_VM_OPTO_ADDNODE_HPP
2 changes: 2 additions & 0 deletions hotspot/src/share/vm/opto/classes.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ macro(LoopLimit)
macro(Mach)
macro(MachProj)
macro(MaxI)
macro(MaxL)
macro(MemBarAcquire)
macro(LoadFence)
macro(MemBarAcquireLock)
Expand All @@ -180,6 +181,7 @@ macro(MemBarVolatile)
macro(MemBarStoreStore)
macro(MergeMem)
macro(MinI)
macro(MinL)
macro(ModD)
macro(ModF)
macro(ModI)
Expand Down
14 changes: 14 additions & 0 deletions hotspot/src/share/vm/opto/connode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -942,6 +942,20 @@ const Type *ConvI2LNode::Value( PhaseTransform *phase ) const {
return tl;
}

Node* ConvI2LNode::Identity(PhaseGVN* phase) {
// If type is in "int" sub-range, we can
// convert I2L(L2I(x)) => x
// since the conversions have no effect.
if (in(1)->Opcode() == Op_ConvL2I) {
Node* x = in(1)->in(1);
const TypeLong* t = phase->type(x)->isa_long();
if (t != NULL && t->_lo >= min_jint && t->_hi <= max_jint) {
return x;
}
}
return this;
}

#ifdef _LP64
static inline bool long_ranges_overlap(jlong lo1, jlong hi1,
jlong lo2, jlong hi2) {
Expand Down
1 change: 1 addition & 0 deletions hotspot/src/share/vm/opto/connode.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,7 @@ class ConvI2LNode : public TypeNode {
virtual int Opcode() const;
virtual const Type *Value( PhaseTransform *phase ) const;
virtual Node *Ideal(PhaseGVN *phase, bool can_reshape);
virtual Node* Identity(PhaseGVN* phase);
virtual uint ideal_reg() const { return Op_RegL; }
};

Expand Down
103 changes: 41 additions & 62 deletions hotspot/src/share/vm/opto/loopTransform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1291,7 +1291,7 @@ void PhaseIdealLoop::do_unroll( IdealLoopTree *loop, Node_List &old_new, bool ad
new_limit = _igvn.intcon(limit->get_int() - stride_con);
set_ctrl(new_limit, C->root());
} else {
// Limit is not constant.
// Limit is not constant. Int subtraction could lead to underflow.
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
Expand All @@ -1303,53 +1303,37 @@ void PhaseIdealLoop::do_unroll( IdealLoopTree *loop, Node_List &old_new, bool ad
Node* opaq_ctrl = get_ctrl(opaq);
limit = new (C) Opaque2Node( C, limit );
register_new_node( limit, opaq_ctrl );

// The Opaque2 node created above (in the case of the first unrolling) hides the type of the loop limit.
// Propagate this precise type information.
limit = new (C) CastIINode(limit, limit_type);
register_new_node(limit, opaq_ctrl);
}
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.
new_limit = new (C) SubINode(limit, stride);
// (1) Convert to long.
Node* limit_l = new (C) ConvI2LNode(limit);
register_new_node(limit_l, get_ctrl(limit));
Node* stride_l = _igvn.longcon(stride_con);
set_ctrl(stride_l, C->root());

// (2) Subtract: compute in long, to prevent underflow.
Node* new_limit_l = new (C) SubLNode(limit_l, stride_l);
register_new_node(new_limit_l, ctrl);

// (3) Clamp to int range, in case we had subtraction underflow.
Node* underflow_clamp_l = _igvn.longcon((stride_con > 0) ? min_jint : max_jint);
set_ctrl(underflow_clamp_l, C->root());
Node* new_limit_no_underflow_l = NULL;
if (stride_con > 0) {
// limit = MaxL(limit - stride, min_jint)
new_limit_no_underflow_l = new (C) MaxLNode(C, new_limit_l, underflow_clamp_l);
} else {
// (limit - stride) may underflow.
// Clamp the adjustment value with MININT or MAXINT:
//
// new_limit = limit-stride
// if (stride > 0)
// new_limit = (limit < new_limit) ? MININT : new_limit;
// else
// new_limit = (limit > new_limit) ? MAXINT : new_limit;
//
BoolTest::mask bt = loop_end->test_trip();
assert(bt == BoolTest::lt || bt == BoolTest::gt, "canonical test is expected");
Node* adj_max = _igvn.intcon((stride_con > 0) ? min_jint : max_jint);
set_ctrl(adj_max, C->root());
Node* old_limit = NULL;
Node* adj_limit = NULL;
Node* bol = limit->is_CMove() ? limit->in(CMoveNode::Condition) : NULL;
if (loop_head->unrolled_count() > 1 &&
limit->is_CMove() && limit->Opcode() == Op_CMoveI &&
limit->in(CMoveNode::IfTrue) == adj_max &&
bol->as_Bool()->_test._test == bt &&
bol->in(1)->Opcode() == Op_CmpI &&
bol->in(1)->in(2) == limit->in(CMoveNode::IfFalse)) {
// Loop was unrolled before.
// Optimize the limit to avoid nested CMove:
// use original limit as old limit.
old_limit = bol->in(1)->in(1);
// Adjust previous adjusted limit.
adj_limit = limit->in(CMoveNode::IfFalse);
adj_limit = new (C) SubINode(adj_limit, stride);
} else {
old_limit = limit;
adj_limit = new (C) SubINode(limit, stride);
}
assert(old_limit != NULL && adj_limit != NULL, "");
register_new_node( adj_limit, ctrl ); // adjust amount
Node* adj_cmp = new (C) CmpINode(old_limit, adj_limit);
register_new_node( adj_cmp, ctrl );
Node* adj_bool = new (C) BoolNode(adj_cmp, bt);
register_new_node( adj_bool, ctrl );
new_limit = new (C) CMoveINode(adj_bool, adj_limit, adj_max, TypeInt::INT);
// limit = MinL(limit - stride, max_jint)
new_limit_no_underflow_l = new (C) MinLNode(C, new_limit_l, underflow_clamp_l);
}
register_new_node(new_limit_no_underflow_l, ctrl);

// (4) Convert back to int.
new_limit = new (C) ConvL2INode(new_limit_no_underflow_l);
register_new_node(new_limit, ctrl);
}
assert(new_limit != NULL, "");
Expand Down Expand Up @@ -1532,6 +1516,9 @@ bool IdealLoopTree::dominates_backedge(Node* ctrl) {
//------------------------------adjust_limit-----------------------------------
// Helper function that computes new loop limit as (rc_limit-offset)/scale
Node* PhaseIdealLoop::adjust_limit(bool is_positive_stride, Node* scale, Node* offset, Node* rc_limit, Node* old_limit, Node* pre_ctrl, bool round) {
Node* old_limit_long = new (C) ConvI2LNode(old_limit);
register_new_node(old_limit_long, pre_ctrl);

Node* sub = new (C) SubLNode(rc_limit, offset);
register_new_node(sub, pre_ctrl);
Node* limit = new (C) DivLNode(NULL, sub, scale);
Expand All @@ -1557,27 +1544,19 @@ Node* PhaseIdealLoop::adjust_limit(bool is_positive_stride, Node* scale, Node* o
// - integer underflow of limit: MAXL chooses old_limit (>= MIN_INT > limit)
// INT() is finally converting the limit back to an integer value.

// We use CMove nodes to implement long versions of min/max (MINL/MAXL).
// We use helper methods for inner MINL/MAXL which return CMoveL nodes to keep a long value for the outer MINL/MAXL comparison:
Node* inner_result_long;
Node* inner_result_long = NULL;
Node* outer_result_long = NULL;
if (is_positive_stride) {
inner_result_long = MaxNode::signed_max(limit, _igvn.longcon(min_jint), TypeLong::LONG, _igvn);
inner_result_long = new (C) MaxLNode(C, limit, _igvn.longcon(min_jint));
outer_result_long = new (C) MinLNode(C, inner_result_long, old_limit_long);
} else {
inner_result_long = MaxNode::signed_min(limit, _igvn.longcon(max_jint), TypeLong::LONG, _igvn);
inner_result_long = new (C) MinLNode(C, limit, _igvn.longcon(max_jint));
outer_result_long = new (C) MaxLNode(C, inner_result_long, old_limit_long);
}
set_subtree_ctrl(inner_result_long);
register_new_node(inner_result_long, pre_ctrl);
register_new_node(outer_result_long, pre_ctrl);

// Outer MINL/MAXL:
// The comparison is done with long values but the result is the converted back to int by using CmovI.
Node* old_limit_long = new (C) ConvI2LNode(old_limit);
register_new_node(old_limit_long, pre_ctrl);
Node* cmp = new (C) CmpLNode(old_limit_long, limit);
register_new_node(cmp, pre_ctrl);
Node* bol = new (C) BoolNode(cmp, is_positive_stride ? BoolTest::gt : BoolTest::lt);
register_new_node(bol, pre_ctrl);
Node* inner_result_int = new (C) ConvL2INode(inner_result_long); // Could under-/overflow but that's fine as comparison was done with CmpL
register_new_node(inner_result_int, pre_ctrl);
limit = new (C) CMoveINode(bol, old_limit, inner_result_int, TypeInt::INT);
limit = new (C) ConvL2INode(outer_result_long);
register_new_node(limit, pre_ctrl);
return limit;
}
Expand Down
16 changes: 15 additions & 1 deletion hotspot/src/share/vm/opto/macro.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2487,7 +2487,9 @@ void PhaseMacroExpand::eliminate_macro_nodes() {
assert(n->Opcode() == Op_LoopLimit ||
n->Opcode() == Op_Opaque1 ||
n->Opcode() == Op_Opaque2 ||
n->Opcode() == Op_Opaque3, "unknown node type in macro list");
n->Opcode() == Op_Opaque3 ||
n->Opcode() == Op_MaxL ||
n->Opcode() == Op_MinL, "unknown node type in macro list");
}
assert(success == (C->macro_count() < old_macro_count), "elimination reduces macro count");
progress = progress || success;
Expand Down Expand Up @@ -2552,6 +2554,18 @@ bool PhaseMacroExpand::expand_macro_nodes() {
_igvn.replace_node(n, repl);
success = true;
#endif
} else if (n->Opcode() == Op_MaxL) {
// Since MaxL and MinL are not implemented in the backend, we expand them to
// a CMoveL construct now. At least until here, the type could be computed
// precisely. CMoveL is not so smart, but we can give it at least the best
// type we know abouot n now.
Node* repl = MaxNode::signed_max(n->in(1), n->in(2), _igvn.type(n), _igvn);
_igvn.replace_node(n, repl);
success = true;
} else if (n->Opcode() == Op_MinL) {
Node* repl = MaxNode::signed_min(n->in(1), n->in(2), _igvn.type(n), _igvn);
_igvn.replace_node(n, repl);
success = true;
}
assert(success == (C->macro_count() < old_macro_count), "elimination reduces macro count");
progress = progress || success;
Expand Down
Loading

1 comment on commit 54f7734

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.