Skip to content

Commit 703d98a

Browse files
mur47x111mur47x111
committed
Enhance range table switch
1 parent f339523 commit 703d98a

File tree

13 files changed

+225
-88
lines changed

13 files changed

+225
-88
lines changed

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

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
/*
2+
* Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation. Oracle designates this
8+
* particular file as subject to the "Classpath" exception as provided
9+
* by Oracle in the LICENSE file that accompanied this code.
10+
*
11+
* This code is distributed in the hope that it will be useful, but WITHOUT
12+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
13+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
14+
* version 2 for more details (a copy is included in the LICENSE file that
15+
* accompanied this code).
16+
*
17+
* You should have received a copy of the GNU General Public License version
18+
* 2 along with this work; if not, write to the Free Software Foundation,
19+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
20+
*
21+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
22+
* or visit www.oracle.com if you need additional information or have any
23+
* questions.
24+
*/
25+
package jdk.graal.compiler.core.test;
26+
27+
import static jdk.graal.compiler.core.common.GraalOptions.AlignJumpTableEntry;
28+
29+
import org.junit.Test;
30+
31+
import jdk.graal.compiler.options.OptionValues;
32+
33+
// Regression test for JDK-8220643 (GR-14583)
34+
public class IntegerSwitchTest extends GraalCompilerTest {
35+
public static int snippetWithoutRange(int arg) {
36+
return switch (arg) {
37+
case 0 -> 0;
38+
case 1 -> -1;
39+
case 2 -> 2;
40+
case 3 -> -3;
41+
case 4 -> 4;
42+
case 5 -> -5;
43+
case 6 -> 6;
44+
case 7 -> -7;
45+
default -> -2;
46+
};
47+
}
48+
49+
@Test
50+
public void testSnippetWithoutRange() {
51+
test("snippetWithoutRange", 2);
52+
test(new OptionValues(getInitialOptions(), AlignJumpTableEntry, false), "snippetWithoutRange", 4);
53+
}
54+
55+
public static int snippetWithRange(int arg) {
56+
return switch (arg & 0x7) {
57+
case 0 -> 0;
58+
case 1 -> -1;
59+
case 2 -> 2;
60+
case 3 -> -3;
61+
case 4 -> 4;
62+
case 5 -> -5;
63+
case 6 -> 6;
64+
case 7 -> -7;
65+
default -> -2;
66+
};
67+
}
68+
69+
@Test
70+
public void testSnippetWithRange() {
71+
test("snippetWithRange", 2);
72+
test(new OptionValues(getInitialOptions(), AlignJumpTableEntry, false), "snippetWithRange", 4);
73+
}
74+
}

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/asm/amd64/AMD64Assembler.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3787,6 +3787,7 @@ protected final void patchJumpTarget(int branch, int branchTarget) {
37873787
int op = getByte(branch);
37883788
// @formatter:off
37893789
assert op == 0xE8 // call
3790+
|| op == 0xFF // jump table
37903791
|| op == 0x00 // jump table entry
37913792
|| op == 0xE9 // jmp
37923793
|| op == 0xEB // short jmp
@@ -3795,7 +3796,10 @@ protected final void patchJumpTarget(int branch, int branchTarget) {
37953796
: "Invalid opcode at patch point branch=" + branch + ", branchTarget=" + branchTarget + ", op=" + op;
37963797
// @formatter:on
37973798

3798-
if (op == 0x00) {
3799+
if (op == 0xFF) {
3800+
int imm32 = branchTarget - (branch + 4);
3801+
emitInt(imm32, branch);
3802+
} else if (op == 0x00) {
37993803
int offsetToJumpTableBase = getShort(branch + 1);
38003804
int jumpTableBase = branch - offsetToJumpTableBase;
38013805
int imm32 = branchTarget - jumpTableBase;

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/core/aarch64/AArch64LIRGenerator.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -566,8 +566,9 @@ protected StrategySwitchOp createStrategySwitchOp(SwitchStrategy strategy, Label
566566
}
567567

568568
@Override
569-
protected void emitRangeTableSwitch(int lowKey, LabelRef defaultTarget, LabelRef[] targets, SwitchStrategy remainingStrategy, LabelRef[] remainingTargets, AllocatableValue key) {
570-
append(new RangeTableSwitchOp(lowKey, defaultTarget, targets, remainingStrategy, remainingTargets, key));
569+
protected void emitRangeTableSwitch(int lowKey, LabelRef defaultTarget, LabelRef[] targets, SwitchStrategy remainingStrategy, LabelRef[] remainingTargets, AllocatableValue key,
570+
boolean inputMayBeOutOfRange) {
571+
append(new RangeTableSwitchOp(lowKey, defaultTarget, targets, remainingStrategy, remainingTargets, key, inputMayBeOutOfRange));
571572
}
572573

573574
@Override

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/core/amd64/AMD64LIRGenerator.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@
7070
import jdk.graal.compiler.lir.LIRValueUtil;
7171
import jdk.graal.compiler.lir.LabelRef;
7272
import jdk.graal.compiler.lir.StandardOp.JumpOp;
73+
import jdk.graal.compiler.lir.StandardOp.NewScratchRegisterOp;
7374
import jdk.graal.compiler.lir.SwitchStrategy;
7475
import jdk.graal.compiler.lir.Variable;
7576
import jdk.graal.compiler.lir.amd64.AMD64AESDecryptOp;
@@ -1159,8 +1160,11 @@ public void emitStrategySwitch(SwitchStrategy strategy, AllocatableValue key, La
11591160
}
11601161

11611162
@Override
1162-
protected void emitRangeTableSwitch(int lowKey, LabelRef defaultTarget, LabelRef[] targets, SwitchStrategy remainingStrategy, LabelRef[] remainingTargets, AllocatableValue key) {
1163-
append(new RangeTableSwitchOp(this, lowKey, defaultTarget, targets, remainingStrategy, remainingTargets, key));
1163+
protected void emitRangeTableSwitch(int lowKey, LabelRef defaultTarget, LabelRef[] targets, SwitchStrategy remainingStrategy, LabelRef[] remainingTargets, AllocatableValue key,
1164+
boolean inputMayBeOutOfRange) {
1165+
AllocatableValue scratch = newVariable(LIRKind.value(target().arch.getWordKind()));
1166+
append(new NewScratchRegisterOp(scratch));
1167+
append(new RangeTableSwitchOp(this, lowKey, defaultTarget, targets, remainingStrategy, remainingTargets, key, scratch, inputMayBeOutOfRange));
11641168
}
11651169

11661170
@Override

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/core/common/GraalOptions.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,9 @@ public final class GraalOptions {
317317
@Option(help = "Alignment in bytes for loop header blocks that have no fall through paths.", type = OptionType.Debug)
318318
public static final OptionKey<Integer> IsolatedLoopHeaderAlignment = new OptionKey<>(32);
319319

320+
@Option(help = "Aligns jump table entries as if they were loop headers.", type = OptionType.Debug)
321+
public static final OptionKey<Boolean> AlignJumpTableEntry = new OptionKey<>(true);
322+
320323
@Option(help = "Evaluates array region equality checks at compile time if the receiver is a constant and the length of the array is less than this value.", type = OptionType.Expert)
321324
public static final OptionKey<Integer> ArrayRegionEqualsConstantLimit = new OptionKey<>(4096);
322325

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/core/gen/NodeLIRBuilder.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -744,8 +744,7 @@ public void emitSwitch(SwitchNode x) {
744744
LIRKind kind = gen.getLIRKind(x.value().stamp(NodeView.DEFAULT));
745745
Value key = gen.emitConstant(kind, x.keyAt(0));
746746
gen.emitCompareBranch(kind.getPlatformKind(), value, key, Condition.EQ, false, getLIRBlock(x.keySuccessor(0)), defaultTarget, probability);
747-
} else if (x instanceof IntegerSwitchNode && x.isSorted()) {
748-
IntegerSwitchNode intSwitch = (IntegerSwitchNode) x;
747+
} else if (x instanceof IntegerSwitchNode intSwitch && x.isSorted()) {
749748
LabelRef[] keyTargets = new LabelRef[keyCount];
750749
JavaConstant[] keyConstants = new JavaConstant[keyCount];
751750
double[] keyProbabilities = new double[keyCount];
@@ -756,7 +755,7 @@ public void emitSwitch(SwitchNode x) {
756755
keyProbabilities[i] = intSwitch.keyProbability(i);
757756
assert keyConstants[i].getJavaKind() == keyKind : Assertions.errorMessage(keyConstants, keyKind);
758757
}
759-
gen.emitStrategySwitch(keyConstants, keyProbabilities, keyTargets, defaultTarget, value);
758+
gen.emitStrategySwitch(keyConstants, keyProbabilities, keyTargets, defaultTarget, value, intSwitch.inputMayBeOutOfRange());
760759
} else {
761760
// keyKind != JavaKind.Int || !x.isSorted()
762761
LabelRef[] keyTargets = new LabelRef[keyCount];

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/lir/StandardOp.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,22 @@ public void emitCode(CompilationResultBuilder crb) {
460460
}
461461
}
462462

463+
public static final class NewScratchRegisterOp extends LIRInstruction {
464+
public static final LIRInstructionClass<NewScratchRegisterOp> TYPE = LIRInstructionClass.create(NewScratchRegisterOp.class);
465+
466+
@Def(OperandFlag.REG) private Value value;
467+
468+
public NewScratchRegisterOp(Value value) {
469+
super(TYPE);
470+
this.value = value;
471+
}
472+
473+
@Override
474+
public void emitCode(CompilationResultBuilder crb) {
475+
// do nothing, just define a value
476+
}
477+
}
478+
463479
@Opcode("SPILLREGISTERS")
464480
public static final class SpillRegistersOp extends LIRInstruction {
465481
public static final LIRInstructionClass<SpillRegistersOp> TYPE = LIRInstructionClass.create(SpillRegistersOp.class);

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/lir/aarch64/AArch64ControlFlow.java‎

Lines changed: 24 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -417,7 +417,10 @@ public static final class RangeTableSwitchOp extends AArch64BlockEndOp {
417417
private final LabelRef[] remainingTargets;
418418
@Alive({REG}) protected AllocatableValue key;
419419

420-
public RangeTableSwitchOp(int lowKey, LabelRef defaultTarget, LabelRef[] targets, SwitchStrategy remainingStrategy, LabelRef[] remainingTargets, AllocatableValue key) {
420+
private final boolean inputMayBeOutOfRange;
421+
422+
public RangeTableSwitchOp(int lowKey, LabelRef defaultTarget, LabelRef[] targets, SwitchStrategy remainingStrategy, LabelRef[] remainingTargets, AllocatableValue key,
423+
boolean inputMayBeOutOfRange) {
421424
super(TYPE);
422425
this.lowKey = lowKey;
423426
assert defaultTarget != null;
@@ -426,6 +429,7 @@ public RangeTableSwitchOp(int lowKey, LabelRef defaultTarget, LabelRef[] targets
426429
this.remainingStrategy = remainingStrategy;
427430
this.remainingTargets = remainingTargets;
428431
this.key = key;
432+
this.inputMayBeOutOfRange = inputMayBeOutOfRange;
429433
}
430434

431435
@Override
@@ -443,26 +447,28 @@ public void emitCode(CompilationResultBuilder crb, AArch64MacroAssembler masm) {
443447
keyOffsetReg = scratch2;
444448
}
445449

446-
int interval = highKey - lowKey;
447-
if (AArch64MacroAssembler.isComparisonImmediate(interval)) {
448-
masm.compare(32, keyOffsetReg, interval);
449-
} else {
450-
masm.mov(scratch1, interval);
451-
masm.cmp(32, keyOffsetReg, scratch1);
452-
}
450+
if (inputMayBeOutOfRange || remainingStrategy != null) {
451+
int interval = highKey - lowKey;
452+
if (AArch64MacroAssembler.isComparisonImmediate(interval)) {
453+
masm.compare(32, keyOffsetReg, interval);
454+
} else {
455+
masm.mov(scratch1, interval);
456+
masm.cmp(32, keyOffsetReg, scratch1);
457+
}
453458

454-
Label outOfRangeLabel = defaultTarget.label();
455-
if (remainingStrategy != null) {
456-
Label remainingLabel = new Label();
457-
outOfRangeLabel = remainingLabel;
459+
Label outOfRangeLabel = defaultTarget.label();
460+
if (remainingStrategy != null) {
461+
Label remainingLabel = new Label();
462+
outOfRangeLabel = remainingLabel;
458463

459-
crb.getLIR().addSlowPath(this, () -> {
460-
masm.bind(remainingLabel);
461-
new StrategySwitchOp(remainingStrategy, remainingTargets, defaultTarget, key, AArch64LIRGenerator::toIntConditionFlag).emitCode(crb, masm);
462-
});
464+
crb.getLIR().addSlowPath(this, () -> {
465+
masm.bind(remainingLabel);
466+
new StrategySwitchOp(remainingStrategy, remainingTargets, defaultTarget, key, AArch64LIRGenerator::toIntConditionFlag).emitCode(crb, masm);
467+
});
468+
}
469+
// Jump to outOfRangeLabel if index is not within the jump table
470+
masm.branchConditionally(ConditionFlag.HI, outOfRangeLabel);
463471
}
464-
// Jump to outOfRangeLabel if index is not within the jump table
465-
masm.branchConditionally(ConditionFlag.HI, outOfRangeLabel);
466472

467473
emitJumpTable(crb, masm, keyOffsetReg, scratch1, scratch2, lowKey, highKey, Arrays.stream(targets).map(LabelRef::label));
468474
}

0 commit comments

Comments
 (0)