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 #60

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -1218,12 +1218,18 @@ void PhaseCFG::verify() const {
for (uint k = 0; k < n->req(); k++) {
Node *def = n->in(k);
if (def && def != n) {
assert(get_block_for_node(def) || def->is_Con(), "must have block; constants for debug info ok");
// Verify that instructions in the block is in correct order.
Block* def_block = get_block_for_node(def);
assert(def_block || def->is_Con(), "must have block; constants for debug info ok");
// Verify that all definitions dominate their uses (except for virtual
// instructions merging multiple definitions).
assert(n->is_Root() || n->is_Region() || n->is_Phi() || n->is_MachMerge() ||
def_block->dominates(block),
"uses must be dominated by definitions");
// Verify that instructions in the block are in correct order.
// Uses must follow their definition if they are at the same block.
// Mostly done to check that MachSpillCopy nodes are placed correctly
// when CreateEx node is moved in build_ifg_physical().
if (get_block_for_node(def) == block && !(block->head()->is_Loop() && n->is_Phi()) &&
if (def_block == block && !(block->head()->is_Loop() && n->is_Phi()) &&
// See (+++) comment in reg_split.cpp
!(n->jvms() != NULL && n->jvms()->is_monitor_use(k))) {
bool is_loop = false;
@@ -1239,6 +1245,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,14 +1390,40 @@ 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.
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);
if (n->outcnt() == 0 &&
(!n->is_Proj() || n->as_Proj()->in(0)->outcnt() == 1) ){
// 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* 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) {
// 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(NULL, C);
sb->remove_node(k);
new_cnt--;
}
// Now remove the node itself.
n->disconnect_inputs(NULL, C);
sb->remove_node(j);
new_cnt--;
@@ -114,6 +114,8 @@ void PhaseChaitin::insert_proj( Block *b, uint i, Node *spill, uint maxlrg ) {
// Do not insert between a call and his Catch
if( b->get_node(i)->is_Catch() ) {
// Put the instruction at the top of the fall-thru block.
// This assumes that the instruction is not used in the other exception
// blocks. Global code motion is responsible for maintaining this invariant.
// Find the fall-thru projection
while( 1 ) {
const CatchProjNode *cp = b->get_node(++i)->as_CatchProj();
@@ -0,0 +1,65 @@
/*
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

package compiler.exceptions;

/**
* @test
* @bug 8263227
* @summary Tests that users of return values from exception-throwing method
* calls are not duplicated in the call's exception path. The second
* run with a variable seed is added for test robustness.
* @library /test/lib /
* @run main/othervm -XX:+IgnoreUnrecognizedVMOptions
* -XX:+UnlockDiagnosticVMOptions
* -Xbatch -XX:+StressGCM -XX:StressSeed=0
* -XX:+VerifyRegisterAllocator
* -XX:CompileCommand=dontinline,java.lang.Integer::*
* compiler.exceptions.TestSpilling
* @run main/othervm -XX:+IgnoreUnrecognizedVMOptions
* -XX:+UnlockDiagnosticVMOptions
* -Xbatch -XX:+StressGCM
* -XX:+VerifyRegisterAllocator
* -XX:CompileCommand=dontinline,java.lang.Integer::*
* compiler.exceptions.TestSpilling
*/

public class TestSpilling {

public static void test() {
int a = Integer.valueOf(42).intValue();
// After global code motion, the logic below should only be placed in
// the fall-through path of java.lang.Integer::intValue(). Otherwise,
// live range splitting might create uses without reaching definitions
// if 'a' is spilled.
int b = (((a & 0x0000F000)) + 1);
int c = a / b + ((a % b > 0) ? 1 : 0);
}

public static void main(String[] args) {
for (int i = 0; i < 10_000; i++) {
test();
}
}

}