8260420: C2 compilation fails with assert(found_sfpt) failed: no node in loop that's not input to safepoint
Reviewed-by: kvn, roland, chagedorn
diff --git a/src/hotspot/share/opto/loopopts.cpp b/src/hotspot/share/opto/loopopts.cpp
index cc1f955..8f2ee6a 100644
--- a/src/hotspot/share/opto/loopopts.cpp
+++ b/src/hotspot/share/opto/loopopts.cpp
@@ -1430,6 +1430,7 @@
// control, then the cloning of n is a pointless exercise, because
// GVN will ensure that we end up where we started.
if (!n->is_Load() || late_load_ctrl != n_ctrl) {
+ Node* outer_loop_clone = NULL;
for (DUIterator_Last jmin, j = n->last_outs(jmin); j >= jmin; ) {
Node *u = n->last_out(j); // Clone private computation per use
_igvn.rehash_node_delayed(u);
@@ -1477,16 +1478,25 @@
IdealLoopTree* x_loop = get_loop(x_ctrl);
Node* x_head = x_loop->_head;
- if (x_head->is_Loop() && (x_head->is_OuterStripMinedLoop() || x_head->as_Loop()->is_strip_mined()) && is_dominator(n_ctrl, x_head)) {
- // Anti dependence analysis is sometimes too
- // conservative: a store in the outer strip mined loop
- // can prevent a load from floating out of the outer
- // strip mined loop but the load may not be referenced
- // from the safepoint: loop strip mining verification
- // code reports a problem in that case. Make sure the
- // load is not moved in the outer strip mined loop in
- // that case.
- x_ctrl = x_head->as_Loop()->skip_strip_mined()->in(LoopNode::EntryControl);
+ if (x_head->is_Loop() && (x_head->is_OuterStripMinedLoop() || x_head->as_Loop()->is_strip_mined())) {
+ if (is_dominator(n_ctrl, x_head)) {
+ // Anti dependence analysis is sometimes too
+ // conservative: a store in the outer strip mined loop
+ // can prevent a load from floating out of the outer
+ // strip mined loop but the load may not be referenced
+ // from the safepoint: loop strip mining verification
+ // code reports a problem in that case. Make sure the
+ // load is not moved in the outer strip mined loop in
+ // that case.
+ x_ctrl = x_head->as_Loop()->skip_strip_mined()->in(LoopNode::EntryControl);
+ } else if (x_head->is_OuterStripMinedLoop()) {
+ // Do not add duplicate LoadNodes to the outer strip mined loop
+ if (outer_loop_clone != NULL) {
+ _igvn.replace_node(x, outer_loop_clone);
+ continue;
+ }
+ outer_loop_clone = x;
+ }
}
assert(dom_depth(n_ctrl) <= dom_depth(x_ctrl), "n is later than its clone");
@@ -1503,8 +1513,7 @@
// to fold a StoreP and an AddP together (as part of an
// address expression) and the AddP and StoreP have
// different controls.
- BarrierSetC2* bs = BarrierSet::barrier_set()->barrier_set_c2();
- if (!x->is_Load() && !x->is_DecodeNarrowPtr() && !x->is_AddP() && !bs->is_gc_barrier_node(x)) {
+ if (!x->is_Load() && !x->is_DecodeNarrowPtr()) {
_igvn._worklist.yank(x);
}
}
diff --git a/test/hotspot/jtreg/compiler/loopopts/TestSplitIfPinnedLoadInStripMinedLoop.java b/test/hotspot/jtreg/compiler/loopopts/TestSplitIfPinnedLoadInStripMinedLoop.java
index fb6a299..839413f 100644
--- a/test/hotspot/jtreg/compiler/loopopts/TestSplitIfPinnedLoadInStripMinedLoop.java
+++ b/test/hotspot/jtreg/compiler/loopopts/TestSplitIfPinnedLoadInStripMinedLoop.java
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2020, 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
@@ -23,7 +23,8 @@
/*
* @test
- * @bug 8249607
+ * @bug 8249607 8260420
+ * @library /test/lib
* @summary A LoadNode is pinned in split_if_with_blocks_post() on a loop exit node x that is part of a strip mined loop. It has a late control y outside
the outer strip mined loop. After pre-main-post, the dominator chain of y does not include x anymore resulting in an assertion failure.
* @run main/othervm -Xbatch -XX:CompileCommand=compileonly,compiler.loopopts.TestSplitIfPinnedLoadInStripMinedLoop::*
@@ -31,6 +32,8 @@
*/
package compiler.loopopts;
+import jdk.test.lib.Asserts;
+
public class TestSplitIfPinnedLoadInStripMinedLoop {
public boolean bFld = false;
@@ -39,7 +42,7 @@
public static float fFld= 6.0f;
public static int iArrFld[] = new int[400];
- public void test() {
+ public void test1() {
int x = 7;
int y = 8;
int a = 9;
@@ -97,10 +100,72 @@
}
}
}
+
+ static class MyClass {
+ int x = 42;
+ }
+
+ int res = 0;
+
+ // The obj1.x load has two uses: The 'res' store and the return. After cloning, both loads end up in the
+ // OuterStripMinedLoop which triggers an assert in LoopNode::verify_strip_mined:
+ // assert(found_sfpt) failed: no node in loop that's not input to safepoint
+ int test2(MyClass obj1, MyClass obj2) {
+ for (int i = 0; i < 10; ++i) {
+ for (int j = 0; j < 10; ++j) {
+ obj2.x = 42; // Prevents obj1.x load from floating up because obj2 could alias obj1
+ res = obj1.x;
+ }
+ for (int j = 0; j < 10_000; ++j) {
+ }
+ }
+ return res;
+ }
+
+ // Same as test2 but with reference to outer loop induction variable 'i' and different order of instructions.
+ // Triggers an assert in PhaseIdealLoop::build_loop_late_post_work if loop strip mining verification is disabled:
+ // assert(false) failed: Bad graph detected in build_loop_late
+ int test3(MyClass obj1, MyClass obj2) {
+ for (int i = 0; i < 10; ++i) {
+ for (int j = 0; j < 10; ++j) {
+ res = obj1.x + i;
+ obj2.x = 42;
+ }
+ for (int j = 0; j < 10_000; ++j) {
+ }
+ }
+ return res;
+ }
+
+ // Same as test2 but with reference to inner loop induction variable 'j' and different order of instructions.
+ // Triggers an assert in PhaseCFG::insert_anti_dependences if loop strip mining verification is disabled:
+ // assert(!LCA_orig->dominates(pred_block) || early->dominates(pred_block)) failed: early is high enough
+ int test4(MyClass obj1, MyClass obj2) {
+ for (int i = 0; i < 10; ++i) {
+ for (int j = 0; j < 10; ++j) {
+ res = obj1.x + j;
+ obj2.x = 42;
+ }
+ for (int j = 0; j < 10_000; ++j) {
+ }
+ }
+ return res;
+ }
+
public static void main(String[] strArr) {
TestSplitIfPinnedLoadInStripMinedLoop t = new TestSplitIfPinnedLoadInStripMinedLoop();
+ MyClass obj = new MyClass();
for (int i = 0; i < 10; i++) {
- t.test();
+ t.test1();
+ int res = t.test2(obj, obj);
+ Asserts.assertEquals(res, t.res);
+ Asserts.assertEquals(res, 42);
+ res = t.test3(obj, obj);
+ Asserts.assertEquals(res, t.res);
+ Asserts.assertEquals(res, 51);
+ res = t.test4(obj, obj);
+ Asserts.assertEquals(res, t.res);
+ Asserts.assertEquals(res, 51);
}
}
}
