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

[GR-56000] Don't allow IntegerExactOverflowNode as condition in ConditionalNode #10576

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2009, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2009, 2025, 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
Expand Down Expand Up @@ -84,6 +84,7 @@
import jdk.graal.compiler.nodes.spi.SimplifierTool;
import jdk.graal.compiler.nodes.spi.SwitchFoldable;
import jdk.graal.compiler.nodes.util.GraphUtil;
import jdk.graal.compiler.replacements.nodes.arithmetic.IntegerExactOverflowNode;
import jdk.vm.ci.code.CodeUtil;
import jdk.vm.ci.meta.Constant;
import jdk.vm.ci.meta.JavaConstant;
Expand Down Expand Up @@ -1706,6 +1707,13 @@ private ValueNode canonicalizeConditionalCascade(SimplifierTool tool, ValueNode
if (trueValue.getStackKind() != JavaKind.Int && trueValue.getStackKind() != JavaKind.Long) {
return null;
}
if (condition() instanceof IntegerExactOverflowNode) {
/*
* An exact overflow node is tightly coupled to the if node that uses it. It must not be
* used as the condition in a conditional node.
*/
return null;
}
if (isSafeConditionalInput(trueValue, trueSuccessor) && isSafeConditionalInput(falseValue, falseSuccessor)) {
return graph().unique(new ConditionalNode(condition(), trueValue, falseValue));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2019, 2025, 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
Expand Down Expand Up @@ -31,9 +31,12 @@
import java.util.List;

import jdk.graal.compiler.core.common.type.Stamp;
import jdk.graal.compiler.graph.Node;
import jdk.graal.compiler.graph.NodeClass;
import jdk.graal.compiler.nodeinfo.NodeInfo;
import jdk.graal.compiler.nodes.AbstractBeginNode;
import jdk.graal.compiler.nodes.FixedGuardNode;
import jdk.graal.compiler.nodes.GuardNode;
import jdk.graal.compiler.nodes.IfNode;
import jdk.graal.compiler.nodes.LogicNode;
import jdk.graal.compiler.nodes.NodeView;
Expand Down Expand Up @@ -65,6 +68,14 @@ public ValueNode getY() {
return y;
}

@Override
protected boolean verifyNode() {
for (Node usage : usages()) {
assertTrue(usage instanceof GuardNode || usage instanceof FixedGuardNode || usage instanceof IfNode, "exact overflow node must only be used by guards or ifs, found: %s", usage);
}
return super.verifyNode();
}

/**
* Make sure the overflow detection nodes have the same order of inputs as the exact arithmetic
* nodes.
Expand Down Expand Up @@ -119,5 +130,12 @@ public void simplify(SimplifierTool tool) {

coupledNodes.forEach(n -> n.replaceAndDelete(split));
}
if (hasNoUsages()) {
/*
* We don't want GVN during canonicalization to find this node and give it usages again,
* since the canonicalizer would not call this simplify method again.
*/
safeDelete();
}
}
}