Skip to content

Commit 388bce8

Browse files
d-kozakd-kozak
committed
[GR-61590] [GR-61408] [GR-60880] Add support for handling unsigned comparisons in points-to analysis.
PullRequest: graal/19886
2 parents acb8816 + 85faf2c commit 388bce8

File tree

5 files changed

+47
-56
lines changed

5 files changed

+47
-56
lines changed

‎substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/BooleanPrimitiveCheckTypeFlow.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,19 +39,22 @@ public class BooleanPrimitiveCheckTypeFlow extends BooleanCheckTypeFlow {
3939
private final TypeFlow<?> left;
4040
private final TypeFlow<?> right;
4141
private final PrimitiveComparison comparison;
42+
private final boolean isUnsigned;
4243

43-
public BooleanPrimitiveCheckTypeFlow(BytecodePosition position, AnalysisType declaredType, TypeFlow<?> left, TypeFlow<?> right, PrimitiveComparison comparison) {
44+
public BooleanPrimitiveCheckTypeFlow(BytecodePosition position, AnalysisType declaredType, TypeFlow<?> left, TypeFlow<?> right, PrimitiveComparison comparison, boolean isUnsigned) {
4445
super(position, declaredType);
4546
this.left = left;
4647
this.right = right;
4748
this.comparison = comparison;
49+
this.isUnsigned = isUnsigned;
4850
}
4951

5052
private BooleanPrimitiveCheckTypeFlow(PointsToAnalysis bb, MethodFlowsGraph methodFlows, BooleanPrimitiveCheckTypeFlow original) {
5153
super(original, methodFlows);
5254
this.left = methodFlows.lookupCloneOf(bb, original.left);
5355
this.right = methodFlows.lookupCloneOf(bb, original.right);
5456
this.comparison = original.comparison;
57+
this.isUnsigned = original.isUnsigned;
5558
}
5659

5760
@Override
@@ -86,6 +89,6 @@ public TypeState eval(PointsToAnalysis bb) {
8689
}
8790
assert leftState.isPrimitive() : left;
8891
assert rightState.isPrimitive() : right;
89-
return convertToBoolean(bb, TypeState.filter(leftState, comparison, rightState), TypeState.filter(leftState, comparison.negate(), rightState));
92+
return convertToBoolean(bb, TypeState.filter(leftState, comparison, rightState, isUnsigned), TypeState.filter(leftState, comparison.negate(), rightState, isUnsigned));
9093
}
9194
}

‎substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/MethodTypeFlowBuilder.java

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@
107107
import jdk.graal.compiler.nodes.calc.ConditionalNode;
108108
import jdk.graal.compiler.nodes.calc.FloatEqualsNode;
109109
import jdk.graal.compiler.nodes.calc.FloatLessThanNode;
110+
import jdk.graal.compiler.nodes.calc.IntegerBelowNode;
110111
import jdk.graal.compiler.nodes.calc.IntegerEqualsNode;
111112
import jdk.graal.compiler.nodes.calc.IntegerLowerThanNode;
112113
import jdk.graal.compiler.nodes.calc.IntegerTestNode;
@@ -1028,7 +1029,7 @@ public TypeFlowBuilder<?> lookup(ValueNode n) {
10281029
var y = lookup(equalsNode.getY());
10291030
var type = getNodeType(equalsNode);
10301031
result = TypeFlowBuilder.create(bb, method, getPredicate(), node, BooleanPrimitiveCheckTypeFlow.class, () -> {
1031-
var flow = new BooleanPrimitiveCheckTypeFlow(AbstractAnalysisEngine.sourcePosition(node), type, x.get(), y.get(), PrimitiveComparison.EQ);
1032+
var flow = new BooleanPrimitiveCheckTypeFlow(AbstractAnalysisEngine.sourcePosition(node), type, x.get(), y.get(), PrimitiveComparison.EQ, false);
10321033
flowsGraph.addMiscEntryFlow(flow);
10331034
return flow;
10341035
});
@@ -1037,9 +1038,10 @@ public TypeFlowBuilder<?> lookup(ValueNode n) {
10371038
} else if (node instanceof IntegerLowerThanNode lowerThan) {
10381039
var x = lookup(lowerThan.getX());
10391040
var y = lookup(lowerThan.getY());
1041+
var isUnsigned = lowerThan instanceof IntegerBelowNode;
10401042
var type = getNodeType(lowerThan);
10411043
result = TypeFlowBuilder.create(bb, method, getPredicate(), node, BooleanPrimitiveCheckTypeFlow.class, () -> {
1042-
var flow = new BooleanPrimitiveCheckTypeFlow(AbstractAnalysisEngine.sourcePosition(node), type, x.get(), y.get(), PrimitiveComparison.LT);
1044+
var flow = new BooleanPrimitiveCheckTypeFlow(AbstractAnalysisEngine.sourcePosition(node), type, x.get(), y.get(), PrimitiveComparison.LT, isUnsigned);
10431045
flowsGraph.addMiscEntryFlow(flow);
10441046
return flow;
10451047
});
@@ -1311,7 +1313,7 @@ private TypeFlowBuilder<?> uniqueReturnFlowBuilder(ReturnNode node) {
13111313
return returnBuilder;
13121314
}
13131315

1314-
private void handleCompareNode(ValueNode source, CompareNode condition, PrimitiveComparison comparison) {
1316+
private void handleCompareNode(ValueNode source, CompareNode condition, PrimitiveComparison comparison, boolean isUnsigned) {
13151317
var xNode = typeFlowUnproxify(condition.getX());
13161318
var yNode = typeFlowUnproxify(condition.getY());
13171319
var xConstant = xNode.isConstant();
@@ -1320,7 +1322,7 @@ private void handleCompareNode(ValueNode source, CompareNode condition, Primitiv
13201322
var yFlow = state.lookup(yNode);
13211323
if (!xConstant) {
13221324
var leftFlowBuilder = TypeFlowBuilder.create(bb, method, state.getPredicate(), source, PrimitiveFilterTypeFlow.class, () -> {
1323-
var flow = new PrimitiveFilterTypeFlow(AbstractAnalysisEngine.sourcePosition(source), xFlow.get().declaredType, xFlow.get(), yFlow.get(), comparison);
1325+
var flow = new PrimitiveFilterTypeFlow(AbstractAnalysisEngine.sourcePosition(source), xFlow.get().declaredType, xFlow.get(), yFlow.get(), comparison, isUnsigned);
13241326
if (yConstant) {
13251327
flowsGraph.addNodeFlow(source, flow);
13261328
}
@@ -1334,7 +1336,7 @@ private void handleCompareNode(ValueNode source, CompareNode condition, Primitiv
13341336
}
13351337
if (!yConstant) {
13361338
var rightFlowBuilder = TypeFlowBuilder.create(bb, method, state.getPredicate(), source, PrimitiveFilterTypeFlow.class, () -> {
1337-
var flow = new PrimitiveFilterTypeFlow(AbstractAnalysisEngine.sourcePosition(source), yFlow.get().declaredType, yFlow.get(), xFlow.get(), comparison.flip());
1339+
var flow = new PrimitiveFilterTypeFlow(AbstractAnalysisEngine.sourcePosition(source), yFlow.get().declaredType, yFlow.get(), xFlow.get(), comparison.flip(), isUnsigned);
13381340
flowsGraph.addNodeFlow(source, flow);
13391341
return flow;
13401342

@@ -1359,9 +1361,10 @@ private void handleUnaryOpLogicNode(UnaryOpLogicNode condition, TypeFlowBuilder<
13591361
private void handleCondition(ValueNode source, LogicNode condition, boolean isTrue) {
13601362
if (state.usePredicates()) {
13611363
if (condition instanceof IntegerLowerThanNode lowerThan) {
1362-
handleCompareNode(source, lowerThan, isTrue ? PrimitiveComparison.LT : PrimitiveComparison.GE);
1364+
var isUnsigned = lowerThan instanceof IntegerBelowNode;
1365+
handleCompareNode(source, lowerThan, isTrue ? PrimitiveComparison.LT : PrimitiveComparison.GE, isUnsigned);
13631366
} else if (condition instanceof IntegerEqualsNode equalsNode) {
1364-
handleCompareNode(source, equalsNode, isTrue ? PrimitiveComparison.EQ : PrimitiveComparison.NEQ);
1367+
handleCompareNode(source, equalsNode, isTrue ? PrimitiveComparison.EQ : PrimitiveComparison.NEQ, false);
13651368
}
13661369
}
13671370
if (condition instanceof IsNullNode nullCheck) {

‎substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/PrimitiveFilterTypeFlow.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,19 +43,22 @@ public class PrimitiveFilterTypeFlow extends TypeFlow<BytecodePosition> {
4343
private final TypeFlow<?> left;
4444
private final TypeFlow<?> right;
4545
private final PrimitiveComparison comparison;
46+
private final boolean isUnsigned;
4647

47-
public PrimitiveFilterTypeFlow(BytecodePosition position, AnalysisType declaredType, TypeFlow<?> left, TypeFlow<?> right, PrimitiveComparison comparison) {
48+
public PrimitiveFilterTypeFlow(BytecodePosition position, AnalysisType declaredType, TypeFlow<?> left, TypeFlow<?> right, PrimitiveComparison comparison, boolean isUnsigned) {
4849
super(position, declaredType);
4950
this.left = left;
5051
this.right = right;
5152
this.comparison = comparison;
53+
this.isUnsigned = isUnsigned;
5254
}
5355

5456
private PrimitiveFilterTypeFlow(PointsToAnalysis bb, MethodFlowsGraph methodFlows, PrimitiveFilterTypeFlow original) {
5557
super(original, methodFlows);
5658
this.left = methodFlows.lookupCloneOf(bb, original.left);
5759
this.right = methodFlows.lookupCloneOf(bb, original.right);
5860
this.comparison = original.comparison;
61+
this.isUnsigned = original.isUnsigned;
5962
}
6063

6164
@Override
@@ -85,7 +88,7 @@ private TypeState eval(PointsToAnalysis bb) {
8588
var rightState = right.getOutputState(bb);
8689
assert leftState.isPrimitive() || leftState.isEmpty() : left;
8790
assert rightState.isPrimitive() || rightState.isEmpty() : right;
88-
return TypeState.filter(leftState, comparison, rightState);
91+
return TypeState.filter(leftState, comparison, rightState, isUnsigned);
8992
}
9093

9194
public PrimitiveComparison getComparison() {

‎substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/typestate/PrimitiveTypeState.java

Lines changed: 25 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@
3333
import com.oracle.graal.pointsto.meta.AnalysisType;
3434
import com.oracle.graal.pointsto.util.AnalysisError;
3535

36+
import jdk.graal.compiler.core.common.calc.UnsignedMath;
37+
3638
/**
3739
* Represents type state for primitive values. These type states can be either concrete constants
3840
* represented by {@link PrimitiveConstantTypeState} or {@link AnyPrimitiveTypeState} that represent
@@ -72,18 +74,6 @@ public TypeState forUnion(PrimitiveTypeState right) {
7274
return AnyPrimitiveTypeState.SINGLETON;
7375
}
7476

75-
/** Returns a type state filtered with respect to the comparison and right. */
76-
public TypeState filter(PrimitiveComparison comparison, PrimitiveTypeState right) {
77-
return switch (comparison) {
78-
case EQ -> forEquals(right);
79-
case NEQ -> forNotEquals(right);
80-
case LT -> forLessThan(right);
81-
case GE -> forGreaterOrEqual(right);
82-
case GT -> forGreaterThan(right);
83-
case LE -> forLessOrEqual(right);
84-
};
85-
}
86-
8777
public TypeState forEquals(PrimitiveTypeState right) {
8878
if (this instanceof PrimitiveConstantTypeState thisConstant) {
8979
if (right instanceof PrimitiveConstantTypeState rightConstant && thisConstant.getValue() != rightConstant.getValue()) {
@@ -100,39 +90,31 @@ public TypeState forEquals(PrimitiveTypeState right) {
10090
throw AnalysisError.shouldNotReachHere("Combination not covered, this=" + this + ". right=" + right);
10191
}
10292

103-
public TypeState forNotEquals(PrimitiveTypeState right) {
104-
if (this instanceof PrimitiveConstantTypeState thisConstant && right instanceof PrimitiveConstantTypeState rightConstant && thisConstant.getValue() == rightConstant.getValue()) {
105-
return forEmpty();
106-
}
107-
return this;
108-
}
109-
110-
public TypeState forLessThan(PrimitiveTypeState right) {
111-
if (this instanceof PrimitiveConstantTypeState thisConstant && right instanceof PrimitiveConstantTypeState rightConstant && thisConstant.getValue() >= rightConstant.getValue()) {
112-
return forEmpty();
113-
}
114-
return this;
115-
}
116-
117-
public TypeState forGreaterOrEqual(PrimitiveTypeState right) {
118-
if (this instanceof PrimitiveConstantTypeState thisConstant && right instanceof PrimitiveConstantTypeState rightConstant && thisConstant.getValue() < rightConstant.getValue()) {
119-
return forEmpty();
120-
}
121-
return this;
122-
}
123-
124-
public TypeState forLessOrEqual(PrimitiveTypeState right) {
125-
if (this instanceof PrimitiveConstantTypeState thisConstant && right instanceof PrimitiveConstantTypeState rightConstant && thisConstant.getValue() > rightConstant.getValue()) {
126-
return forEmpty();
93+
/** Returns a type state filtered with respect to the comparison and right. */
94+
public TypeState filter(PrimitiveComparison comparison, boolean isUnsigned, PrimitiveTypeState right) {
95+
if (comparison == PrimitiveComparison.EQ) {
96+
/*
97+
* This is the only case where we can improve even if one of the operands is Any, so we
98+
* handle it first.
99+
*/
100+
return forEquals(right);
127101
}
128-
return this;
129-
}
130-
131-
public TypeState forGreaterThan(PrimitiveTypeState right) {
132-
if (this instanceof PrimitiveConstantTypeState thisConstant && right instanceof PrimitiveConstantTypeState rightConstant && thisConstant.getValue() <= rightConstant.getValue()) {
133-
return forEmpty();
102+
if (!(this instanceof PrimitiveConstantTypeState leftConstant && right instanceof PrimitiveConstantTypeState rightConstant)) {
103+
/*
104+
* Apart from EQ, we cannot handle non-constant case, so always return the original
105+
* value.
106+
*/
107+
return this;
134108
}
135-
return this;
109+
boolean filterToEmpty = switch (comparison) {
110+
case NEQ -> leftConstant.getValue() == rightConstant.getValue();
111+
case LT -> isUnsigned ? UnsignedMath.aboveOrEqual(leftConstant.getValue(), rightConstant.getValue()) : leftConstant.getValue() >= rightConstant.getValue();
112+
case GE -> isUnsigned ? UnsignedMath.belowThan(leftConstant.getValue(), rightConstant.getValue()) : leftConstant.getValue() < rightConstant.getValue();
113+
case GT -> isUnsigned ? UnsignedMath.belowOrEqual(leftConstant.getValue(), rightConstant.getValue()) : leftConstant.getValue() <= rightConstant.getValue();
114+
case LE -> isUnsigned ? UnsignedMath.aboveThan(leftConstant.getValue(), rightConstant.getValue()) : leftConstant.getValue() > rightConstant.getValue();
115+
default -> throw AnalysisError.shouldNotReachHere("Unreachable case in switch.");
116+
};
117+
return filterToEmpty ? forEmpty() : this;
136118
}
137119

138120
public abstract boolean canBeTrue();

‎substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/typestate/TypeState.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -274,13 +274,13 @@ public static TypeState forSubtraction(PointsToAnalysis bb, TypeState s1, TypeSt
274274
}
275275

276276
/** Returns a type state representing left filtered with respect to the comparison and right. */
277-
public static TypeState filter(TypeState left, PrimitiveComparison comparison, TypeState right) {
277+
public static TypeState filter(TypeState left, PrimitiveComparison comparison, TypeState right, boolean isUnsigned) {
278278
assert left.isPrimitive() || left.isEmpty() : left;
279279
assert right.isPrimitive() || right.isEmpty() : right;
280280
if (left.isEmpty() || right.isEmpty()) {
281281
return forEmpty();
282282
}
283-
return ((PrimitiveTypeState) left).filter(comparison, (PrimitiveTypeState) right);
283+
return ((PrimitiveTypeState) left).filter(comparison, isUnsigned, (PrimitiveTypeState) right);
284284
}
285285
}
286286

0 commit comments

Comments
 (0)