Skip to content

Commit 8da6491

Browse files
davleopodavleopo
committed
[GR-53900] Loop infra: use correct bit size for overflow checks.
PullRequest: graal/17739
2 parents 08170ef + 0ab5107 commit 8da6491

File tree

4 files changed

+93
-7
lines changed

4 files changed

+93
-7
lines changed

‎compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/LoopFullUnrollTest.java‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
*/
2525
package jdk.graal.compiler.core.test;
2626

27+
import jdk.graal.compiler.api.directives.GraalDirectives;
2728
import jdk.graal.compiler.debug.DebugContext;
2829
import jdk.graal.compiler.debug.DebugDumpScope;
2930
import jdk.graal.compiler.loop.phases.LoopFullUnrollPhase;
@@ -95,4 +96,38 @@ private void test(String snippet, int loopCount) {
9596
throw debug.handle(e);
9697
}
9798
}
99+
100+
public static int snippetFlows() {
101+
int init = Integer.MIN_VALUE;
102+
int step = -1;
103+
int limit = 1;
104+
int phi = init;
105+
while (Integer.MIN_VALUE - phi < limit) {
106+
GraalDirectives.sideEffect();
107+
phi = phi + step;
108+
}
109+
return phi;
110+
}
111+
112+
@Test
113+
public void testFlows() {
114+
test("snippetFlows");
115+
}
116+
117+
public static int snippetFlows2() {
118+
int init = Integer.MAX_VALUE;
119+
int step = -8;
120+
int limit = 8184;
121+
int phi = init;
122+
while (Integer.MIN_VALUE - phi < limit) {
123+
GraalDirectives.sideEffect();
124+
phi = phi + step;
125+
}
126+
return phi;
127+
}
128+
129+
@Test
130+
public void testFlows2() {
131+
test("snippetFlows2");
132+
}
98133
}

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/loop/DerivedOffsetInductionVariable.java‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
*/
2525
package jdk.graal.compiler.nodes.loop;
2626

27+
import jdk.graal.compiler.core.common.type.IntegerStamp;
2728
import jdk.graal.compiler.core.common.type.Stamp;
2829
import jdk.graal.compiler.debug.Assertions;
2930
import jdk.graal.compiler.debug.GraalError;
@@ -34,6 +35,7 @@
3435
import jdk.graal.compiler.nodes.calc.IntegerConvertNode;
3536
import jdk.graal.compiler.nodes.calc.NegateNode;
3637
import jdk.graal.compiler.nodes.calc.SubNode;
38+
import jdk.graal.compiler.phases.common.util.LoopUtility;
3739
import jdk.graal.compiler.replacements.nodes.arithmetic.IntegerExactArithmeticNode;
3840

3941
public class DerivedOffsetInductionVariable extends DerivedInductionVariable {
@@ -102,7 +104,7 @@ public long constantStride() {
102104

103105
private long constantStrideSafe() throws ArithmeticException {
104106
if (value instanceof SubNode && base.valueNode() == value.getY()) {
105-
return Math.multiplyExact(base.constantStride(), -1);
107+
return LoopUtility.multiplyExact(IntegerStamp.getBits(offset.stamp(NodeView.DEFAULT)), base.constantStride(), -1);
106108
}
107109
return base.constantStride();
108110
}
@@ -158,16 +160,18 @@ public ValueNode exitValueNode() {
158160
}
159161

160162
private long opSafe(long b, long o) throws ArithmeticException {
163+
// we can use offset bits in this method because all operands (init, scale, stride and
164+
// extremum) have by construction equal bit sizes
161165
if (value instanceof AddNode) {
162-
return Math.addExact(b, o);
166+
return LoopUtility.addExact(IntegerStamp.getBits(offset.stamp(NodeView.DEFAULT)), b, o);
163167
}
164168
if (value instanceof SubNode) {
165169
if (base.valueNode() == value.getX()) {
166-
return Math.subtractExact(b, o);
170+
return LoopUtility.subtractExact(IntegerStamp.getBits(offset.stamp(NodeView.DEFAULT)), b, o);
167171
} else {
168172
assert base.valueNode() == value.getY() || base instanceof BasicInductionVariable basic && basic.getOp() instanceof IntegerExactArithmeticNode : Assertions.errorMessage(base,
169173
base.valueNode(), value, value.getY());
170-
return Math.subtractExact(b, o);
174+
return LoopUtility.subtractExact(IntegerStamp.getBits(offset.stamp(NodeView.DEFAULT)), b, o);
171175
}
172176
}
173177
throw GraalError.shouldNotReachHereUnexpectedValue(value); // ExcludeFromJacocoGeneratedReport

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/loop/DerivedScaledInductionVariable.java‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import jdk.graal.compiler.nodes.calc.IntegerConvertNode;
3535
import jdk.graal.compiler.nodes.calc.NegateNode;
3636
import jdk.graal.compiler.nodes.util.GraphUtil;
37+
import jdk.graal.compiler.phases.common.util.LoopUtility;
3738

3839
public class DerivedScaledInductionVariable extends DerivedInductionVariable {
3940

@@ -121,7 +122,7 @@ public long constantInit() {
121122
}
122123

123124
private long constantInitSafe() throws ArithmeticException {
124-
return Math.multiplyExact(base.constantInit(), scale.asJavaConstant().asLong());
125+
return opSafe(base.constantInit(), scale.asJavaConstant().asLong());
125126
}
126127

127128
@Override
@@ -130,7 +131,13 @@ public long constantStride() {
130131
}
131132

132133
private long constantStrideSafe() throws ArithmeticException {
133-
return Math.multiplyExact(base.constantStride(), scale.asJavaConstant().asLong());
134+
return opSafe(base.constantStride(), scale.asJavaConstant().asLong());
135+
}
136+
137+
private long opSafe(long a, long b) {
138+
// we can use scale bits here because all operands (init, scale, stride and extremum) have
139+
// by construction equal bit sizes
140+
return LoopUtility.multiplyExact(IntegerStamp.getBits(scale.stamp(NodeView.DEFAULT)), a, b);
134141
}
135142

136143
@Override
@@ -152,7 +159,7 @@ public long constantExtremum() {
152159
}
153160

154161
private long constantExtremumSafe() throws ArithmeticException {
155-
return Math.multiplyExact(base.constantExtremum(), scale.asJavaConstant().asLong());
162+
return opSafe(base.constantExtremum(), scale.asJavaConstant().asLong());
156163
}
157164

158165
@Override

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/phases/common/util/LoopUtility.java‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import jdk.graal.compiler.core.common.cfg.Loop;
3030
import jdk.graal.compiler.core.common.type.IntegerStamp;
3131
import jdk.graal.compiler.core.common.type.Stamp;
32+
import jdk.graal.compiler.debug.Assertions;
3233
import jdk.graal.compiler.debug.DebugContext;
3334
import jdk.graal.compiler.debug.GraalError;
3435
import jdk.graal.compiler.graph.Graph.NodeEvent;
@@ -60,6 +61,45 @@
6061

6162
public class LoopUtility {
6263

64+
public static long addExact(int bits, long a, long b) {
65+
if (bits == 32) {
66+
int ia = (int) a;
67+
int ib = (int) b;
68+
assert ia == a && ib == b : Assertions.errorMessage("Conversions must be lossless", bits, a, b, ia, ib);
69+
return Math.addExact(ia, ib);
70+
} else if (bits == 64) {
71+
return Math.addExact(a, b);
72+
} else {
73+
throw GraalError.shouldNotReachHere("Must be one of java's core datatypes int/long but is " + bits);
74+
}
75+
}
76+
77+
public static long subtractExact(int bits, long a, long b) {
78+
if (bits == 32) {
79+
int ia = (int) a;
80+
int ib = (int) b;
81+
assert ia == a && ib == b : Assertions.errorMessage("Conversions must be lossless", bits, a, b, ia, ib);
82+
return Math.subtractExact(ia, ib);
83+
} else if (bits == 64) {
84+
return Math.subtractExact(a, b);
85+
} else {
86+
throw GraalError.shouldNotReachHere("Must be one of java's core datatypes int/long but is " + bits);
87+
}
88+
}
89+
90+
public static long multiplyExact(int bits, long a, long b) {
91+
if (bits == 32) {
92+
int ia = (int) a;
93+
int ib = (int) b;
94+
assert ia == a && ib == b : Assertions.errorMessage("Conversions must be lossless", bits, a, b, ia, ib);
95+
return Math.multiplyExact(ia, ib);
96+
} else if (bits == 64) {
97+
return Math.multiplyExact(a, b);
98+
} else {
99+
throw GraalError.shouldNotReachHere("Must be one of java's core datatypes int/long but is " + bits);
100+
}
101+
}
102+
63103
public static boolean canTakeAbs(long l, int bits) {
64104
try {
65105
abs(l, bits);

0 commit comments

Comments
 (0)