Skip to content
Permalink
Browse files
8265132: C2 compilation fails with assert "missing precedence edge"
Reviewed-by: roland
Backport-of: 5644c4f
  • Loading branch information
TheRealMDoerr committed Jul 13, 2021
1 parent cbd3b0f commit 8d1e4029100648d6f14090aebdeb6decabfbf30a
Showing with 41 additions and 21 deletions.
  1. +2 −0 src/hotspot/share/opto/block.hpp
  2. +24 −18 src/hotspot/share/opto/gcm.cpp
  3. +15 −3 test/hotspot/jtreg/compiler/uncommontrap/TestNullCheckAntiDependence.java
@@ -507,6 +507,8 @@ class PhaseCFG : public Phase {
bool _trace_opto_pipelining; // tracing flag
#endif

bool unrelated_load_in_store_null_block(Node* store, Node* load);

public:
PhaseCFG(Arena* arena, RootNode* root, Matcher& matcher);

@@ -540,6 +540,28 @@ static Block* memory_early_block(Node* load, Block* early, const PhaseCFG* cfg)
return early;
}

// This function is used by insert_anti_dependences to find unrelated loads for stores in implicit null checks.
bool PhaseCFG::unrelated_load_in_store_null_block(Node* store, Node* load) {
// We expect an anti-dependence edge from 'load' to 'store', except when
// implicit_null_check() has hoisted 'store' above its early block to
// perform an implicit null check, and 'load' is placed in the null
// block. In this case it is safe to ignore the anti-dependence, as the
// null block is only reached if 'store' tries to write to null object and
// 'load' read from non-null object (there is preceding check for that)
// These objects can't be the same.
Block* store_block = get_block_for_node(store);
Block* load_block = get_block_for_node(load);
Node* end = store_block->end();
if (end->is_MachNullCheck() && (end->in(1) == store) && store_block->dominates(load_block)) {
Node* if_true = end->find_out_with(Op_IfTrue);
assert(if_true != NULL, "null check without null projection");
Node* null_block_region = if_true->find_out_with(Op_Region);
assert(null_block_region != NULL, "null check without null region");
return get_block_for_node(null_block_region) == load_block;
}
return false;
}

//--------------------------insert_anti_dependences---------------------------
// A load may need to witness memory that nearby stores can overwrite.
// For each nearby store, either insert an "anti-dependence" edge
@@ -793,7 +815,7 @@ Block* PhaseCFG::insert_anti_dependences(Block* LCA, Node* load, bool verify) {
// will find him on the non_early_stores list and stick him
// with a precedence edge.
// (But, don't bother if LCA is already raised all the way.)
if (LCA != early) {
if (LCA != early && !unrelated_load_in_store_null_block(store, load)) {
store_block->set_raise_LCA_mark(load_index);
must_raise_LCA = true;
non_early_stores.push(store);
@@ -804,23 +826,7 @@ Block* PhaseCFG::insert_anti_dependences(Block* LCA, Node* load, bool verify) {
// Add an anti-dep edge, and squeeze 'load' into the highest block.
assert(store != load->find_exact_control(load->in(0)), "dependence cycle found");
if (verify) {
#ifdef ASSERT
// We expect an anti-dependence edge from 'load' to 'store', except when
// implicit_null_check() has hoisted 'store' above its early block to
// perform an implicit null check, and 'load' is placed in the null
// block. In this case it is safe to ignore the anti-dependence, as the
// null block is only reached if 'store' tries to write to null.
Block* store_null_block = NULL;
Node* store_null_check = store->find_out_with(Op_MachNullCheck);
if (store_null_check != NULL) {
Node* if_true = store_null_check->find_out_with(Op_IfTrue);
assert(if_true != NULL, "null check without null projection");
Node* null_block_region = if_true->find_out_with(Op_Region);
assert(null_block_region != NULL, "null check without null region");
store_null_block = get_block_for_node(null_block_region);
}
#endif
assert(LCA == store_null_block || store->find_edge(load) != -1,
assert(store->find_edge(load) != -1 || unrelated_load_in_store_null_block(store, load),
"missing precedence edge");
} else {
store->add_prec(load);
@@ -25,7 +25,7 @@

/**
* @test
* @bug 8261730
* @bug 8261730 8265132
* @summary Test that no anti-dependence violation is reported between a store
* used as an implicit null check and a load placed in the null block.
* @run main/othervm -XX:-BackgroundCompilation
@@ -40,8 +40,9 @@

private static MyInteger foo = new MyInteger();
private static MyInteger bar = new MyInteger();
private static MyInteger[] global = {new MyInteger()};

static void setFooToZero() {
static void test1() {
for (int i = 0; i < 1; i++) {
// This load is placed in the null block.
foo.val = -bar.val;
@@ -52,10 +53,21 @@ static void setFooToZero() {
}
}

static void test2(MyInteger a, MyInteger b) {
global[0].val = a.val + b.val * 31;
global[0].val = 0;
return;
}

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

for (int i = 0; i < 10_000; i++) {
test2(new MyInteger(), new MyInteger());
}

}

}

1 comment on commit 8d1e402

@openjdk-notifier

This comment has been minimized.

Copy link

@openjdk-notifier openjdk-notifier bot commented on 8d1e402 Jul 13, 2021

Please sign in to comment.