Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8263227: C2: inconsistent spilling due to dead nodes in exception block #3303

Closed
wants to merge 10 commits into from
@@ -1272,6 +1272,14 @@ void PhaseCFG::verify() const {
}
}
}
if (n->is_Proj()) {
assert(j >= 1, "a projection cannot be the first instruction in a block");
Node* pred = block->get_node(j - 1);
Node* parent = n->in(0);
assert(parent != NULL, "projections must have a parent");
assert(pred == parent || (pred->is_Proj() && pred->in(0) == parent),
"projections must follow their parents or other sibling projections");
}
}

j = block->end_idx();
@@ -1390,32 +1390,36 @@ void PhaseCFG::call_catch_cleanup(Block* block) {
}

// If the successor blocks have a CreateEx node, move it back to the top
for(uint i4 = 0; i4 < block->_num_succs; i4++ ) {
for (uint i4 = 0; i4 < block->_num_succs; i4++) {
Block *sb = block->_succs[i4];
uint new_cnt = end - beg;
// Remove any newly created, but dead, nodes. In this context, a dead node
// is either a multi-node with all projections unused, or a non-projection
// single node that is unused. This definition avoids removing unused
// projections of partially-used multi-nodes.
for( uint j = new_cnt; j > 0; j-- ) {
// Remove any newly created, but dead, nodes by traversing their schedule
// backwards. Here, a dead node is a node whose only outputs (if any) are
// unused projections.
for (uint j = new_cnt; j > 0; j--) {
Node *n = sb->get_node(j);
// Individual projections are examined together with all siblings when
// their parent is visited.
if (n->is_Proj()) {
continue;
}
bool dead = true;
for (DUIterator_Fast imax, i = n->fast_outs(imax); i < imax; i++) {
Node* user = n->fast_out(i);
if (!user->is_Proj() || user->outcnt() > 0) {
Node* out = n->fast_out(i);
// n is live if it has a non-projection output or a used projection.
if (!out->is_Proj() || out->outcnt() > 0) {
dead = false;
break;
}
}
if (dead) {
// Remove projections if n is a dead multi-node.
for (uint k = j + n->outcnt(); sb->get_node(k)->is_Proj(); k--) {
assert(sb->get_node(k)->in(0) == n,
"dead projection should correspond to current node");
sb->get_node(k)->disconnect_inputs(C);
// n's only outputs (if any) are unused projections scheduled next to n
// (see PhaseCFG::select()). Remove these projections backwards.
for (uint k = j + n->outcnt(); k > j; k--) {
Node* proj = sb->get_node(k);
assert(proj->is_Proj() && proj->in(0) == n,
"projection should correspond to dead node");
proj->disconnect_inputs(C);
sb->remove_node(k);
This conversation was marked as resolved by robcasloz

This comment has been minimized.

@vnkozlov

vnkozlov Apr 6, 2021
Contributor

If you remove node here then j could be incorrect in sb->remove_node(j) at line #1424

This comment has been minimized.

@robcasloz

robcasloz Apr 8, 2021
Author Contributor

k should always be greater than j, as sb->get_node(k) is a projection of sb->get_node(j). Shouldn't this make the removal safe?

This comment has been minimized.

@vnkozlov

vnkozlov Apr 8, 2021
Contributor

Yes, you are right - users should follow.

new_cnt--;
}
ProTip! Use n and p to navigate between commits in a pull request.