Skip to content

Commit 0fc5386

Browse files
committed
[GR-71120] Canonicalize disjoint or to add
PullRequest: graal/22518
2 parents 7907cf8 + d83098e commit 0fc5386

File tree

4 files changed

+84
-3
lines changed

4 files changed

+84
-3
lines changed

compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/CountedLoopTest.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -587,6 +587,22 @@ public void shiftLargeLong() {
587587
testCounted("shiftLargeLongSnippet");
588588
}
589589

590+
public static Result disjointOrToAddSnippet(int limit) {
591+
Result ret = new Result();
592+
int i;
593+
for (i = 0; GraalDirectives.injectIterationCount(11, (i | 4) < limit); i += 8) {
594+
GraalDirectives.controlFlowAnchor();
595+
ret.extremum = get(InductionVariable::extremumNode, InductionVariable::constantExtremum, InductionVariable::isConstantExtremum, i | 4);
596+
}
597+
ret.exitValue = get(InductionVariable::exitValueNode, i);
598+
return ret;
599+
}
600+
601+
@Test
602+
public void disjointOrToAdd() {
603+
testCounted("disjointOrToAddSnippet", 256);
604+
}
605+
590606
@NodeInfo(cycles = CYCLES_IGNORED, size = SIZE_IGNORED)
591607
private static class IVPropertyNode extends FloatingNode implements LIRLowerable {
592608
public static final NodeClass<IVPropertyNode> TYPE = NodeClass.create(IVPropertyNode.class);

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/core/common/type/IntegerStamp.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -990,6 +990,12 @@ public static boolean negateCanOverflow(IntegerStamp stamp) {
990990
return stamp.lowerBound() == CodeUtil.minValue(stamp.getBits());
991991
}
992992

993+
/** Returns {@code true} if the two stamps cannot have any common set bits. */
994+
public static boolean bitwiseDisjoint(IntegerStamp x, IntegerStamp y) {
995+
GraalError.guarantee(x.getBits() == y.getBits(), "must be compatible: %s / %s", x, y);
996+
return (x.mayBeSet() & y.mayBeSet()) == 0;
997+
}
998+
993999
public static final ArithmeticOpTable OPS = new ArithmeticOpTable(
9941000

9951001
new ArithmeticOpTable.UnaryOp.Neg() {

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/ValuePhiNode.java

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2009, 2023, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2009, 2025, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -26,6 +26,7 @@
2626

2727
import java.util.Map;
2828

29+
import jdk.graal.compiler.core.common.type.IntegerStamp;
2930
import jdk.graal.compiler.core.common.type.Stamp;
3031
import jdk.graal.compiler.core.common.type.StampFactory;
3132
import jdk.graal.compiler.debug.Assertions;
@@ -35,9 +36,11 @@
3536
import jdk.graal.compiler.graph.NodeInputList;
3637
import jdk.graal.compiler.nodeinfo.InputType;
3738
import jdk.graal.compiler.nodeinfo.NodeInfo;
39+
import jdk.graal.compiler.nodes.calc.AddNode;
3840
import jdk.graal.compiler.nodes.spi.LimitedValueProxy;
3941
import jdk.graal.compiler.nodes.type.StampTool;
4042
import jdk.graal.compiler.util.CollectionsUtil;
43+
import jdk.vm.ci.code.CodeUtil;
4144

4245
/**
4346
* Value {@link PhiNode}s merge data flow values at control flow merges.
@@ -91,6 +94,7 @@ public boolean inferStamp() {
9194
valuesStamp = stamp;
9295
}
9396
valuesStamp = tryInferLoopPhiStamp(valuesStamp);
97+
valuesStamp = tryImproveIntegerBits(valuesStamp);
9498
if (stamp.isCompatible(valuesStamp)) {
9599
valuesStamp = stamp.join(valuesStamp);
96100
}
@@ -176,6 +180,54 @@ private Stamp tryInferLoopPhiStamp(Stamp valuesStamp) {
176180
return valuesStamp;
177181
}
178182

183+
/**
184+
* Tries to strengthen an integer-typed loop phi's bitmasks. This is meant for phis like the
185+
* counter in the following loop:
186+
*
187+
* <pre>
188+
* for (int i = 0; i < limit; i += 8) ...
189+
* </pre>
190+
*
191+
* In this loop, {@code i} will always be divisible by 8, even on overflow. We can derive this
192+
* from the may-be-set masks of the inputs: Any low bits that are clear in both the initial
193+
* value and the increment must also be clear in the phi's stamp.
194+
*
195+
* @param valuesStamp a stamp derived from this phi's direct inputs
196+
* @return a stronger stamp than {@code valuesStamp} if lower bits could be cleared;
197+
* {@code valuesStamp} otherwise
198+
*/
199+
private Stamp tryImproveIntegerBits(Stamp valuesStamp) {
200+
if (isAlive() && isLoopPhi() && valuesStamp instanceof IntegerStamp integerStamp && singleBackValueOrThis() instanceof AddNode add && add.getX() == this) {
201+
long valuesMask = integerStamp.mayBeSet();
202+
long initMask = ((IntegerStamp) valueAt(0).stamp(NodeView.DEFAULT)).mayBeSet();
203+
long incrementMask = ((IntegerStamp) add.getY().stamp(NodeView.DEFAULT)).mayBeSet();
204+
int clearLowBits = Long.numberOfTrailingZeros(initMask | incrementMask);
205+
if (Long.numberOfTrailingZeros(valuesMask) < clearLowBits) {
206+
long improvedValuesMask = valuesMask & ~CodeUtil.mask(clearLowBits);
207+
IntegerStamp improvedMaskStamp = IntegerStamp.stampForMask(integerStamp.getBits(), 0, improvedValuesMask);
208+
return valuesStamp.improveWith(improvedMaskStamp);
209+
}
210+
}
211+
return valuesStamp;
212+
}
213+
214+
/**
215+
* Update this phi's stamp by {@linkplain Stamp#tryImproveWith improving it with} the new
216+
* information in the given stamp. This is necessary for cases where we have some high-level
217+
* knowledge about this phi's value that's not expressed in the graph. It's illegal to call
218+
* {@link #setStamp} on value phis because we override {@link #inferStamp()}}.
219+
*
220+
* @return {@code true} if the stamp was updated, {@code false} otherwise
221+
*/
222+
public boolean refineStampWith(Stamp newInformation) {
223+
Stamp improved = stamp.tryImproveWith(newInformation);
224+
if (improved == null) {
225+
return false;
226+
} else {
227+
return updateStamp(improved);
228+
}
229+
}
230+
179231
private static Node stripProxies(Node n) {
180232
if (n instanceof ValueNode value) {
181233
return stripProxies(value);

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/calc/OrNode.java

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2011, 2023, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2011, 2025, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -39,7 +39,6 @@
3939
import jdk.graal.compiler.nodes.spi.CanonicalizerTool;
4040
import jdk.graal.compiler.nodes.spi.NodeLIRBuilderTool;
4141
import jdk.graal.compiler.nodes.util.GraphUtil;
42-
4342
import jdk.vm.ci.meta.Constant;
4443

4544
@NodeInfo(shortName = "|")
@@ -94,6 +93,14 @@ private static ValueNode canonical(OrNode self, BinaryOp<Or> op, ValueNode forX,
9493
return forX;
9594
} else if (((~yStamp.mustBeSet()) & xStamp.mayBeSet()) == 0) {
9695
return forY;
96+
} else if (IntegerStamp.bitwiseDisjoint(xStamp, yStamp)) {
97+
/*
98+
* Representing a disjoint "or" as an add may help us discover induction variables.
99+
* However, don't destroy bit rotate patterns.
100+
*/
101+
if (!(forX instanceof ShiftNode<?> && forY instanceof ShiftNode<?>)) {
102+
return AddNode.create(forX, forY, view);
103+
}
97104
}
98105
}
99106

0 commit comments

Comments
 (0)