stan/jdk-24
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

8251544: CTW: C2 fails with assert(no_dead_loop) failed: dead loop detected

Browse Source 
Reviewed-by: kvn, roland
This commit is contained in:
Christian Hagedorn 2020-10-12 08:18:13 +00:00
parent 13fe054cb9
commit 54bbe76ed0
3 changed files with 253 additions and 31 deletions
src/hotspot/share/opto
cfgnode.cppcfgnode.hpp
test/hotspot/jtreg/compiler/c2
TestDeadDataLoopIGVN.java

73
src/hotspot/share/opto/cfgnode.cpp

@ -315,37 +315,60 @@ static bool check_compare_clipping( bool less_than, IfNode *iff, ConNode *limit,
//------------------------------is_unreachable_region--------------------------
// Find if the Region node is reachable from the root.
bool RegionNode::is_unreachable_region(PhaseGVN *phase) const {
assert(req() == 2, "");
bool RegionNode::is_unreachable_region(const PhaseGVN* phase) {
Node* top = phase->C->top();
assert(req() == 2 || (req() == 3 && in(1) != NULL && in(2) == top), "sanity check arguments");
if (_is_unreachable_region) {
// Return cached result from previous evaluation which should still be valid
assert(is_unreachable_from_root(phase), "walk the graph again and check if its indeed unreachable");
return true;
}
// First, cut the simple case of fallthrough region when NONE of
// region's phis references itself directly or through a data node.
if (is_possible_unsafe_loop(phase)) {
// If we have a possible unsafe loop, check if the region node is actually unreachable from root.
if (is_unreachable_from_root(phase)) {
_is_unreachable_region = true;
return true;
}
}
return false;
}
bool RegionNode::is_possible_unsafe_loop(const PhaseGVN* phase) const {
uint max = outcnt();
uint i;
for (i = 0; i < max; i++) {
Node* phi = raw_out(i);
if (phi != NULL && phi->is_Phi()) {
assert(phase->eqv(phi->in(0), this) && phi->req() == 2, "");
if (phi->outcnt() == 0)
Node* n = raw_out(i);
if (n != NULL && n->is_Phi()) {
PhiNode* phi = n->as_Phi();
assert(phase->eqv(phi->in(0), this), "sanity check phi");
if (phi->outcnt() == 0) {
continue; // Safe case - no loops
}
if (phi->outcnt() == 1) {
Node* u = phi->raw_out(0);
// Skip if only one use is an other Phi or Call or Uncommon trap.
// It is safe to consider this case as fallthrough.
if (u != NULL && (u->is_Phi() || u->is_CFG()))
if (u != NULL && (u->is_Phi() || u->is_CFG())) {
continue;
}
}
// Check when phi references itself directly or through an other node.
if (phi->as_Phi()->simple_data_loop_check(phi->in(1)) >= PhiNode::Unsafe)
if (phi->as_Phi()->simple_data_loop_check(phi->in(1)) >= PhiNode::Unsafe) {
break; // Found possible unsafe data loop.
}
}
}
if (i >= max)
if (i >= max) {
return false; // An unsafe case was NOT found - don't need graph walk.
}
return true;
}
// Unsafe case - check if the Region node is reachable from root.
bool RegionNode::is_unreachable_from_root(const PhaseGVN* phase) const {
ResourceMark rm;
Node_List nstack;
VectorSet visited;
@ -367,7 +390,6 @@ bool RegionNode::is_unreachable_region(PhaseGVN *phase) const {
}
}
}
return true; // The Region node is unreachable - it is dead.
}
@ -1929,16 +1951,7 @@ Node *PhiNode::Ideal(PhaseGVN *phase, bool can_reshape) {
// Determine if this input is backedge of a loop.
// (Skip new phis which have no uses and dead regions).
if (outcnt() > 0 && r->in(0) != NULL) {
// First, take the short cut when we know it is a loop and
// the EntryControl data path is dead.
// Loop node may have only one input because entry path
// is removed in PhaseIdealLoop::Dominators().
assert(!r->is_Loop() || r->req() <= 3, "Loop node should have 3 or less inputs");
bool is_loop = (r->is_Loop() && r->req() == 3);
// Then, check if there is a data loop when phi references itself directly
// or through other data nodes.
if ((is_loop && !uin->eqv_uncast(in(LoopNode::EntryControl))) ||
(!is_loop && is_unsafe_data_reference(uin))) {
if (is_data_loop(r->as_Region(), uin, phase)) {
// Break this data loop to avoid creation of a dead loop.
if (can_reshape) {
return top;
@ -2377,6 +2390,22 @@ Node *PhiNode::Ideal(PhaseGVN *phase, bool can_reshape) {
return progress; // Return any progress
}
bool PhiNode::is_data_loop(RegionNode* r, Node* uin, const PhaseGVN* phase) {
// First, take the short cut when we know it is a loop and the EntryControl data path is dead.
// The loop node may only have one input because the entry path was removed in PhaseIdealLoop::Dominators().
// Then, check if there is a data loop when the phi references itself directly or through other data nodes.
assert(!r->is_Loop() || r->req() <= 3, "Loop node should have 3 or less inputs");
const bool is_loop = (r->is_Loop() && r->req() == 3);
const Node* top = phase->C->top();
if (is_loop) {
return !uin->eqv_uncast(in(LoopNode::EntryControl));
} else {
// We have a data loop either with an unsafe data reference or if a region is unreachable.
return is_unsafe_data_reference(uin)
|| (r->req() == 3 && (r->in(1) != top && r->in(2) == top && r->is_unreachable_region(phase)));
}
}
//------------------------------is_tripcount-----------------------------------
bool PhiNode::is_tripcount() const {
return (in(0) != NULL && in(0)->is_CountedLoop() &&

25
src/hotspot/share/opto/cfgnode.hpp

@ -64,15 +64,20 @@ class PhaseIdealLoop;
// correspond 1-to-1 with RegionNode inputs. The zero input of a PhiNode is
// the RegionNode, and the zero input of the RegionNode is itself.
class RegionNode : public Node {
private:
bool _is_unreachable_region;
bool is_possible_unsafe_loop(const PhaseGVN* phase) const;
bool is_unreachable_from_root(const PhaseGVN* phase) const;
public:
// Node layout (parallels PhiNode):
enum { Region, // Generally points to self.
Control // Control arcs are [1..len)
};
RegionNode( uint required ) : Node(required) {
RegionNode(uint required) : Node(required), _is_unreachable_region(false) {
init_class_id(Class_Region);
init_req(0,this);
init_req(0, this);
}
Node* is_copy() const {
@ -84,18 +89,19 @@ public:
PhiNode* has_phi() const; // returns an arbitrary phi user, or NULL
PhiNode* has_unique_phi() const; // returns the unique phi user, or NULL
// Is this region node unreachable from root?
bool is_unreachable_region(PhaseGVN *phase) const;
bool is_unreachable_region(const PhaseGVN* phase);
virtual int Opcode() const;
virtual bool pinned() const { return (const Node *)in(0) == this; }
virtual bool is_CFG () const { return true; }
virtual uint hash() const { return NO_HASH; } // CFG nodes do not hash
virtual uint size_of() const { return sizeof(*this); }
virtual bool pinned() const { return (const Node*)in(0) == this; }
virtual bool is_CFG() const { return true; }
virtual uint hash() const { return NO_HASH; } // CFG nodes do not hash
virtual bool depends_only_on_test() const { return false; }
virtual const Type *bottom_type() const { return Type::CONTROL; }
virtual const Type* bottom_type() const { return Type::CONTROL; }
virtual const Type* Value(PhaseGVN* phase) const;
virtual Node* Identity(PhaseGVN* phase);
virtual Node *Ideal(PhaseGVN *phase, bool can_reshape);
virtual Node* Ideal(PhaseGVN* phase, bool can_reshape);
virtual const RegMask &out_RegMask() const;
bool try_clean_mem_phi(PhaseGVN *phase);
bool try_clean_mem_phi(PhaseGVN* phase);
bool optimize_trichotomy(PhaseIterGVN* igvn);
};
@ -135,6 +141,7 @@ class PhiNode : public TypeNode {
// Determine if CMoveNode::is_cmove_id can be used at this join point.
Node* is_cmove_id(PhaseTransform* phase, int true_path);
bool wait_for_region_igvn(PhaseGVN* phase);
bool is_data_loop(RegionNode* r, Node* uin, const PhaseGVN* phase);
public:
// Node layout (parallels RegionNode):

186
test/hotspot/jtreg/compiler/c2/TestDeadDataLoopIGVN.java Normal file

@ -0,0 +1,186 @@
/*
* Copyright (c) 2020, 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.
*/
/*
* @test
* @bug 8251544
* @summary A dead data loop in dying code is not correctly removed resulting in unremovable data nodes.
* @requires vm.compiler2.enabled
* @library /test/lib /
* @modules java.base/jdk.internal.misc
* java.management
* java.base/jdk.internal.access
* java.base/jdk.internal.reflect
*
* @build sun.hotspot.WhiteBox
* @run driver ClassFileInstaller sun.hotspot.WhiteBox
* @run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI -XX:-TieredCompilation -Xbatch
* compiler.c2.TestDeadDataLoopIGVN
*/
package compiler.c2;
import java.lang.reflect.Method;
import java.util.HashMap;
import java.util.List;
import java.util.LinkedList;
import java.util.ArrayList;
import java.util.Vector;
import java.util.ListIterator;
import jdk.internal.access.SharedSecrets;
import jdk.internal.misc.Unsafe;
import jdk.internal.reflect.ConstantPool;
import java.net.URL;
import java.net.URLClassLoader;
import java.lang.reflect.Executable;
import compiler.whitebox.CompilerWhiteBoxTest;
import sun.hotspot.WhiteBox;
public class TestDeadDataLoopIGVN {
private static final WhiteBox WB = WhiteBox.getWhiteBox();
private static final int TIERED_STOP_AT_LEVEL = WB.getIntxVMFlag("TieredStopAtLevel").intValue();
private static final Unsafe UNSAFE = Unsafe.getUnsafe();
// The original test only failed with CTW due to different inlining and virtual call decisions compared to an
// execution with -Xcomp. This test adapts the behavior of CTW and compiles the methods with the Whitebox API
// in order to reproduce the bug.
public static void main(String[] strArr) throws Exception {
// Required to get the same inlining/virtual call decisions as for CTW
callSomeMethods(new ArrayList<String>());
callSomeMethods(new Vector<String>());
if (TIERED_STOP_AT_LEVEL != CompilerWhiteBoxTest.COMP_LEVEL_FULL_OPTIMIZATION) {
throw new RuntimeException("Sanity check if C2 is available");
}
// To trigger the assertion, we only need to compile Test
compileClass(Test.class);
}
private static void callSomeMethods(List<String> list) {
list.add("bla");
list.add("foo");
ListIterator<String> it = list.listIterator();
it.hasNext();
for (String s : list) {
s.charAt(0);
}
}
// Adaptation from CTW to compile and deoptimize the same methods
private static void compileClass(Class<?> aClass) throws Exception {
aClass = Class.forName(aClass.getCanonicalName(), true, aClass.getClassLoader());
ConstantPool constantPool = SharedSecrets.getJavaLangAccess().getConstantPool(aClass);
preloadClasses(constantPool);
UNSAFE.ensureClassInitialized(aClass);
WB.enqueueInitializerForCompilation(aClass, 4); // Level 4 for C2
for (Executable method : aClass.getDeclaredConstructors()) {
WB.deoptimizeMethod(method);
WB.enqueueMethodForCompilation(method, 4);
WB.deoptimizeMethod(method);
}
for (Executable method : aClass.getDeclaredMethods()) {
WB.deoptimizeMethod(method);
WB.enqueueMethodForCompilation(method, 4);
WB.deoptimizeMethod(method);
}
}
private static void preloadClasses(ConstantPool constantPool) throws Exception {
for (int i = 0, n = constantPool.getSize(); i < n; ++i) {
try {
constantPool.getClassAt(i);
} catch (IllegalArgumentException ignore) {
}
}
}
}
// The actual class that failed by executing it with CTW
class Test {
public static A a = new A();
Test() {
LinkedList<A> l = new LinkedList<A>();
for (int i = 0; i < 34; i++) {
A instance = new A();
instance.id = i;
l.add(instance);
}
test(l, 34);
}
public void test(LinkedList<A> list, int max) {
Integer[] numbers = new Integer[max + 1];
A[] numbers2 = new A[max + 1];
int n = 0;
ListIterator<A> it = list.listIterator();
while (it.hasNext()) {
A b = it.next();
numbers[b.get()] = n;
numbers2[n] = b;
n++;
}
Integer[] iArr = new Integer[max + 1];
A a = getA();
Integer x = numbers[a.get()];
iArr[x] = x;
boolean flag = true;
while (flag) {
flag = false;
it = list.listIterator(34);
while (it.hasPrevious()) {
A b = it.previous();
if (b == a) {
continue;
}
}
}
HashMap<A, A> map = new HashMap<A, A>();
for (Integer i = 0; i < max - 34; i++) {
map.put(numbers2[i], numbers2[iArr[i]]);
}
}
public A getA() {
return a;
}
}
// Helper class
class A {
int id;
public int get() {
return id;
}
}