Skip to content
This repository has been archived by the owner on Aug 27, 2022. It is now read-only.
/ lanai Public archive

Commit

Permalink
8237859: C2: Crash when loads float above range check
Browse files Browse the repository at this point in the history 
Fix control edges of predicates to data nodes when creating pre/main/post loops.

Reviewed-by: neliasso, thartmann, roland
  • Loading branch information
@chhagedorn
chhagedorn committed Mar 25, 2020
1 parent 9a6038f commit c01e986
Show file tree
Hide file tree
Showing 4 changed files with 231 additions and 31 deletions.
16 changes: 8 additions & 8 deletions src/hotspot/share/opto/loopPredicate.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1207,7 +1207,7 @@ bool PhaseIdealLoop::loop_predication_impl_helper(IdealLoopTree *loop, ProjNode*
new_predicate_proj = upper_bound_proj;

if (iff->is_RangeCheck()) {
new_predicate_proj = insert_skeleton_predicate(iff, loop, proj, predicate_proj, upper_bound_proj, scale, offset, init, limit, stride, rng, overflow, reason);
new_predicate_proj = insert_initial_skeleton_predicate(iff, loop, proj, predicate_proj, upper_bound_proj, scale, offset, init, limit, stride, rng, overflow, reason);
}

#ifndef PRODUCT
Expand Down Expand Up @@ -1238,13 +1238,13 @@ bool PhaseIdealLoop::loop_predication_impl_helper(IdealLoopTree *loop, ProjNode*
// of the main loop induction variable. Make a copy of the predicates
// here with an opaque node as a place holder for the value (will be
// updated by PhaseIdealLoop::clone_skeleton_predicate()).
ProjNode* PhaseIdealLoop::insert_skeleton_predicate(IfNode* iff, IdealLoopTree *loop,
ProjNode* proj, ProjNode *predicate_proj,
ProjNode* upper_bound_proj,
int scale, Node* offset,
Node* init, Node* limit, jint stride,
Node* rng, bool &overflow,
Deoptimization::DeoptReason reason) {
ProjNode* PhaseIdealLoop::insert_initial_skeleton_predicate(IfNode* iff, IdealLoopTree *loop,
ProjNode* proj, ProjNode *predicate_proj,
ProjNode* upper_bound_proj,
int scale, Node* offset,
Node* init, Node* limit, jint stride,
Node* rng, bool &overflow,
Deoptimization::DeoptReason reason) {
assert(proj->_con && predicate_proj->_con, "not a range check?");
Node* opaque_init = new Opaque1Node(C, init);
register_new_node(opaque_init, upper_bound_proj);
Expand Down
72 changes: 62 additions & 10 deletions src/hotspot/share/opto/loopTransform.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1120,6 +1120,19 @@ Node* PhaseIdealLoop::cast_incr_before_loop(Node* incr, Node* ctrl, Node* loop)
return NULL;
}

#ifdef ASSERT
void PhaseIdealLoop::ensure_zero_trip_guard_proj(Node* node, bool is_main_loop) {
assert(node->is_IfProj(), "must be the zero trip guard If node");
Node* zer_bol = node->in(0)->in(1);
assert(zer_bol != NULL && zer_bol->is_Bool(), "must be Bool");
Node* zer_cmp = zer_bol->in(1);
assert(zer_cmp != NULL && zer_cmp->Opcode() == Op_CmpI, "must be CmpI");
// For the main loop, the opaque node is the second input to zer_cmp, for the post loop it's the first input node
Node* zer_opaq = zer_cmp->in(is_main_loop ? 2 : 1);
assert(zer_opaq != NULL && zer_opaq->Opcode() == Op_Opaque1, "must be Opaque1");
}
#endif

// Make a copy of the skeleton range check predicates before the main
// loop and set the initial value of loop as input. After unrolling,
// the range of values for the induction variable in the main loop can
Expand All @@ -1128,10 +1141,16 @@ Node* PhaseIdealLoop::cast_incr_before_loop(Node* incr, Node* ctrl, Node* loop)
// CastII/ConvI2L nodes cause some data paths to die. For consistency,
// the control paths must die too but the range checks were removed by
// predication. The range checks that we add here guarantee that they do.
void PhaseIdealLoop::duplicate_predicates_helper(Node* predicate, Node* start, Node* end,
void PhaseIdealLoop::copy_skeleton_predicates_to_main_loop_helper(Node* predicate, Node* start, Node* end,
IdealLoopTree* outer_loop, LoopNode* outer_main_head,
uint dd_main_head) {
uint dd_main_head, const uint idx_before_pre_post,
const uint idx_after_post_before_pre, Node* zero_trip_guard_proj_main,
Node* zero_trip_guard_proj_post, const Node_List &old_new) {
if (predicate != NULL) {
#ifdef ASSERT
ensure_zero_trip_guard_proj(zero_trip_guard_proj_main, true);
ensure_zero_trip_guard_proj(zero_trip_guard_proj_post, false);
#endif
IfNode* iff = predicate->in(0)->as_If();
ProjNode* uncommon_proj = iff->proj_out(1 - predicate->as_Proj()->_con);
Node* rgn = uncommon_proj->unique_ctrl_out();
Expand All @@ -1147,13 +1166,36 @@ void PhaseIdealLoop::duplicate_predicates_helper(Node* predicate, Node* start, N
break;
if (iff->in(1)->Opcode() == Op_Opaque4) {
assert(skeleton_predicate_has_opaque(iff), "unexpected");
// Clone the predicate twice and initialize one with the initial
// Clone the skeleton predicate twice and initialize one with the initial
// value of the loop induction variable. Leave the other predicate
// to be initialized when increasing the stride during loop unrolling.
prev_proj = clone_skeleton_predicate(iff, start, predicate, uncommon_proj, current_proj, outer_loop, prev_proj);
assert(skeleton_predicate_has_opaque(prev_proj->in(0)->as_If()) == (start->Opcode() == Op_Opaque1), "");
prev_proj = clone_skeleton_predicate(iff, end, predicate, uncommon_proj, current_proj, outer_loop, prev_proj);
assert(skeleton_predicate_has_opaque(prev_proj->in(0)->as_If()) == (end->Opcode() == Op_Opaque1), "");

// Rewire any control inputs from the cloned skeleton predicates down to the main and post loop for data nodes that are part of the
// main loop (and were cloned to the pre and post loop).
for (DUIterator i = predicate->outs(); predicate->has_out(i); i++) {
Node* loop_node = predicate->out(i);
Node* pre_loop_node = old_new[loop_node->_idx];
// Change the control if 'loop_node' is part of the main loop. If there is an old->new mapping and the index of
// 'pre_loop_node' is greater than idx_before_pre_post, then we know that 'loop_node' was cloned and is part of
// the main loop (and 'pre_loop_node' is part of the pre loop).
if (!loop_node->is_CFG() && (pre_loop_node != NULL && pre_loop_node->_idx > idx_after_post_before_pre)) {
// 'loop_node' is a data node and part of the main loop. Rewire the control to the projection of the zero-trip guard if node
// of the main loop that is immediately preceding the cloned predicates.
_igvn.replace_input_of(loop_node, 0, zero_trip_guard_proj_main);
--i;
} else if (loop_node->_idx > idx_before_pre_post && loop_node->_idx < idx_after_post_before_pre) {
// 'loop_node' is a data node and part of the post loop. Rewire the control to the projection of the zero-trip guard if node
// of the post loop that is immediately preceding the post loop header node (there are no cloned predicates for the post loop).
assert(pre_loop_node == NULL, "a node belonging to the post loop should not have an old_new mapping at this stage");
_igvn.replace_input_of(loop_node, 0, zero_trip_guard_proj_post);
--i;
}
}

// Remove the skeleton predicate from the pre-loop
_igvn.replace_input_of(iff, 1, _igvn.intcon(1));
}
Expand Down Expand Up @@ -1278,9 +1320,11 @@ Node* PhaseIdealLoop::clone_skeleton_predicate(Node* iff, Node* value, Node* pre
return proj;
}

void PhaseIdealLoop::duplicate_predicates(CountedLoopNode* pre_head, Node* start, Node* end,
void PhaseIdealLoop::copy_skeleton_predicates_to_main_loop(CountedLoopNode* pre_head, Node* start, Node* end,
IdealLoopTree* outer_loop, LoopNode* outer_main_head,
uint dd_main_head) {
uint dd_main_head, const uint idx_before_pre_post,
const uint idx_after_post_before_pre, Node* zero_trip_guard_proj_main,
Node* zero_trip_guard_proj_post, const Node_List &old_new) {
if (UseLoopPredicate) {
Node* entry = pre_head->in(LoopNode::EntryControl);
Node* predicate = NULL;
Expand All @@ -1296,8 +1340,12 @@ void PhaseIdealLoop::duplicate_predicates(CountedLoopNode* pre_head, Node* start
}
}
predicate = find_predicate_insertion_point(entry, Deoptimization::Reason_predicate);
duplicate_predicates_helper(predicate, start, end, outer_loop, outer_main_head, dd_main_head);
duplicate_predicates_helper(profile_predicate, start, end, outer_loop, outer_main_head, dd_main_head);
copy_skeleton_predicates_to_main_loop_helper(predicate, start, end, outer_loop, outer_main_head, dd_main_head,
idx_before_pre_post, idx_after_post_before_pre, zero_trip_guard_proj_main,
zero_trip_guard_proj_post, old_new);
copy_skeleton_predicates_to_main_loop_helper(profile_predicate, start, end, outer_loop, outer_main_head, dd_main_head,
idx_before_pre_post, idx_after_post_before_pre, zero_trip_guard_proj_main,
zero_trip_guard_proj_post, old_new);
}
}

Expand Down Expand Up @@ -1347,8 +1395,10 @@ void PhaseIdealLoop::insert_pre_post_loops(IdealLoopTree *loop, Node_List &old_n
}

// Add the post loop
const uint idx_before_pre_post = Compile::current()->unique();
CountedLoopNode *post_head = NULL;
Node *main_exit = insert_post_loop(loop, old_new, main_head, main_end, incr, limit, post_head);
const uint idx_after_post_before_pre = Compile::current()->unique();

//------------------------------
// Step B: Create Pre-Loop.
Expand Down Expand Up @@ -1446,7 +1496,9 @@ void PhaseIdealLoop::insert_pre_post_loops(IdealLoopTree *loop, Node_List &old_n
assert(castii != NULL, "no castII inserted");
Node* opaque_castii = new Opaque1Node(C, castii);
register_new_node(opaque_castii, outer_main_head->in(LoopNode::EntryControl));
duplicate_predicates(pre_head, castii, opaque_castii, outer_loop, outer_main_head, dd_main_head);
assert(post_head->in(1)->is_IfProj(), "must be zero-trip guard If node projection of the post loop");
copy_skeleton_predicates_to_main_loop(pre_head, castii, opaque_castii, outer_loop, outer_main_head, dd_main_head,
idx_before_pre_post, idx_after_post_before_pre, min_taken, post_head->in(1), old_new);

// Step B4: Shorten the pre-loop to run only 1 iteration (for now).
// RCE and alignment may change this later.
Expand Down Expand Up @@ -1732,7 +1784,7 @@ bool IdealLoopTree::is_invariant(Node* n) const {
return !is_member(_phase->get_loop(n_c));
}

void PhaseIdealLoop::update_skeleton_predicates(Node* ctrl, CountedLoopNode* loop_head, Node* init, int stride_con) {
void PhaseIdealLoop::update_main_loop_skeleton_predicates(Node* ctrl, CountedLoopNode* loop_head, Node* init, int stride_con) {
// Search for skeleton predicates and update them according to the new stride
Node* entry = ctrl;
Node* prev_proj = ctrl;
Expand Down Expand Up @@ -1840,7 +1892,7 @@ void PhaseIdealLoop::do_unroll(IdealLoopTree *loop, Node_List &old_new, bool adj
assert(old_trip_count > 1 &&
(!adjust_min_trip || stride_p <= (1<<3)*loop_head->unrolled_count()), "sanity");

update_skeleton_predicates(ctrl, loop_head, init, stride_con);
update_main_loop_skeleton_predicates(ctrl, loop_head, init, stride_con);

// Adjust loop limit to keep valid iterations number after unroll.
// Use (limit - stride) instead of (((limit - init)/stride) & (-2))*stride
Expand Down
32 changes: 19 additions & 13 deletions src/hotspot/share/opto/loopnode.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -783,14 +783,20 @@ class PhaseIdealLoop : public PhaseTransform {
}

Node* cast_incr_before_loop(Node* incr, Node* ctrl, Node* loop);
void duplicate_predicates_helper(Node* predicate, Node* start, Node* end, IdealLoopTree* outer_loop,
LoopNode* outer_main_head, uint dd_main_head);
void duplicate_predicates(CountedLoopNode* pre_head, Node* start, Node* end, IdealLoopTree* outer_loop,
LoopNode* outer_main_head, uint dd_main_head);

#ifdef ASSERT
void ensure_zero_trip_guard_proj(Node* node, bool is_main_loop);
#endif
void copy_skeleton_predicates_to_main_loop_helper(Node* predicate, Node* start, Node* end, IdealLoopTree* outer_loop, LoopNode* outer_main_head,
uint dd_main_head, const uint idx_before_pre_post, const uint idx_after_post_before_pre,
Node* zero_trip_guard_proj_main, Node* zero_trip_guard_proj_post, const Node_List &old_new);
void copy_skeleton_predicates_to_main_loop(CountedLoopNode* pre_head, Node* start, Node* end, IdealLoopTree* outer_loop, LoopNode* outer_main_head,
uint dd_main_head, const uint idx_before_pre_post, const uint idx_after_post_before_pre,
Node* zero_trip_guard_proj_main, Node* zero_trip_guard_proj_post, const Node_List &old_new);
Node* clone_skeleton_predicate(Node* iff, Node* value, Node* predicate, Node* uncommon_proj,
Node* current_proj, IdealLoopTree* outer_loop, Node* prev_proj);
Node* current_proj, IdealLoopTree* outer_loop, Node* prev_proj);
bool skeleton_predicate_has_opaque(IfNode* iff);
void update_skeleton_predicates(Node* ctrl, CountedLoopNode* loop_head, Node* init, int stride_con);
void update_main_loop_skeleton_predicates(Node* ctrl, CountedLoopNode* loop_head, Node* init, int stride_con);
void insert_loop_limit_check(ProjNode* limit_check_proj, Node* cmp_limit, Node* bol);

public:
Expand Down Expand Up @@ -1154,13 +1160,13 @@ class PhaseIdealLoop : public PhaseTransform {
void loop_predication_follow_branches(Node *c, IdealLoopTree *loop, float loop_trip_cnt,
PathFrequency& pf, Node_Stack& stack, VectorSet& seen,
Node_List& if_proj_list);
ProjNode* insert_skeleton_predicate(IfNode* iff, IdealLoopTree *loop,
ProjNode* proj, ProjNode *predicate_proj,
ProjNode* upper_bound_proj,
int scale, Node* offset,
Node* init, Node* limit, jint stride,
Node* rng, bool& overflow,
Deoptimization::DeoptReason reason);
ProjNode* insert_initial_skeleton_predicate(IfNode* iff, IdealLoopTree *loop,
ProjNode* proj, ProjNode *predicate_proj,
ProjNode* upper_bound_proj,
int scale, Node* offset,
Node* init, Node* limit, jint stride,
Node* rng, bool& overflow,
Deoptimization::DeoptReason reason);
Node* add_range_check_predicate(IdealLoopTree* loop, CountedLoopNode* cl,
Node* predicate_proj, int scale_con, Node* offset,
Node* limit, jint stride_con, Node* value);
Expand Down
Loading

0 comments on commit c01e986

Please sign in to comment.