Skip to content

Commit e808b93

Browse files
committed
[GR-71246] Fix deopt loops in generated boxing elimination code.
PullRequest: graal/22607
2 parents 82ae059 + 6b8a635 commit e808b93

File tree

9 files changed

+261
-130
lines changed

9 files changed

+261
-130
lines changed

truffle/src/com.oracle.truffle.api.bytecode.test/src/com/oracle/truffle/api/bytecode/test/BoxingEliminationTest.java

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2183,6 +2183,38 @@ public void testAndSingle() {
21832183
assertStable(quickenings, node, 1L);
21842184
}
21852185

2186+
@Test
2187+
public void testNonBEableOperand() {
2188+
// return arg0 + arg1
2189+
BoxingEliminationTestRootNode node = (BoxingEliminationTestRootNode) parse(b -> {
2190+
b.beginRoot();
2191+
b.beginReturn();
2192+
b.beginAddWithNonBEableOperands();
2193+
b.emitLoadArgument(0);
2194+
b.emitLoadArgument(1);
2195+
b.endAddWithNonBEableOperands();
2196+
b.endReturn();
2197+
b.endRoot();
2198+
}).getRootNode();
2199+
2200+
assertInstructions(node,
2201+
"load.argument",
2202+
"load.argument",
2203+
"c.AddWithNonBEableOperands",
2204+
"return");
2205+
2206+
node.getCallTarget().call(42, 3.14f);
2207+
2208+
assertInstructions(node,
2209+
"load.argument$Int",
2210+
"load.argument",
2211+
"c.AddWithNonBEableOperands$IntFloat",
2212+
"return");
2213+
2214+
var quickenings = assertQuickenings(node, 3, 1);
2215+
assertStable(quickenings, node, 123, 4.56f);
2216+
}
2217+
21862218
@GenerateBytecode(languageClass = BytecodeDSLTestLanguage.class, //
21872219
enableYield = true, enableSerialization = true, //
21882220
enableQuickening = true, //
@@ -2651,6 +2683,23 @@ public static Object doInt(int constant1, Object dynamic, int constant2, @Cached
26512683
}
26522684
}
26532685

2686+
@Operation
2687+
static final class AddWithNonBEableOperands {
2688+
/**
2689+
* Regression test: the code for boxing elimination would incorrectly try to boxing
2690+
* eliminate the float operand because another specialization had a BE-able operand at
2691+
* the same operand index.
2692+
*/
2693+
@Specialization
2694+
public static Object doIntFloat(int x, float y) {
2695+
return x + y;
2696+
}
2697+
2698+
@Specialization
2699+
public static Object doFloatInt(float x, int y) {
2700+
return x + y;
2701+
}
2702+
}
26542703
}
26552704

26562705
}

truffle/src/com.oracle.truffle.dsl.processor/src/com/oracle/truffle/dsl/processor/bytecode/generator/BytecodeDSLNodeGeneratorPlugs.java

Lines changed: 108 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -143,13 +143,31 @@ public ChildExecutionResult createExecuteChild(FlatNodeGenFactory factory, CodeT
143143
b.string(targetValue.getName(), " = ");
144144

145145
int index = execution.getIndex();
146-
boolean throwsUnexpectedResult = buildChildExecution(b, frameState, stackFrame(), index);
146+
boolean throwsUnexpectedResult = buildChildExecution(b, frameState, stackFrame(), index, targetValue.getTypeMirror());
147147

148148
return new ChildExecutionResult(b.build(), throwsUnexpectedResult);
149149
}
150150

151-
public boolean canBoxingEliminateType(NodeExecutionData currentExecution, TypeMirror type) {
152-
return model.isBoxingEliminated(type);
151+
public boolean canEliminateTypeGuard(NodeExecutionData currentExecution, TypeMirror type) {
152+
int operandIndex = currentExecution.getIndex() - instruction.signature.constantOperandsBeforeCount;
153+
int operandCount = instruction.signature.dynamicOperandCount;
154+
if (operandIndex < 0) {
155+
// can always be eliminated as they are always precise
156+
return true;
157+
} else if (operandIndex < operandCount) {
158+
if (!instruction.isQuickening()) {
159+
return false;
160+
}
161+
162+
final TypeMirror instructionType = instruction.signature.getDynamicOperandType(operandIndex);
163+
if (ElementUtils.typeEquals(instructionType, type) || instruction.nodeData.getTypeSystem().lookupCast(instructionType, type) != null) {
164+
return model.isBoxingEliminated(type);
165+
}
166+
return false;
167+
} else {
168+
// can always be eliminated as they are always precise
169+
return true;
170+
}
153171
}
154172

155173
public void beforeCallSpecialization(FlatNodeGenFactory nodeFactory, CodeTreeBuilder builder, FrameState frameState,
@@ -161,8 +179,8 @@ public void beforeCallSpecialization(FlatNodeGenFactory nodeFactory, CodeTreeBui
161179
}
162180
}
163181

164-
private boolean buildChildExecution(CodeTreeBuilder b, FrameState frameState, String frame, int specializationIndex) {
165-
int operandIndex = specializationIndex;
182+
private boolean buildChildExecution(CodeTreeBuilder b, FrameState frameState, String frame, int index, TypeMirror targetType) {
183+
int operandIndex = index;
166184
if (operandIndex < instruction.signature.constantOperandsBeforeCount) {
167185
ConstantOperandModel constantOperand = instruction.operation.constantOperands.before().get(operandIndex);
168186
InstructionImmediate imm = instruction.constantOperandImmediates.get(constantOperand);
@@ -175,80 +193,105 @@ private boolean buildChildExecution(CodeTreeBuilder b, FrameState frameState, St
175193
operandIndex -= instruction.signature.constantOperandsBeforeCount;
176194
int operandCount = instruction.signature.dynamicOperandCount;
177195
if (operandIndex < operandCount) {
178-
TypeMirror specializedType = instruction.signature.getSpecializedType(operandIndex);
179-
TypeMirror genericType = instruction.signature.getGenericType(operandIndex);
180-
TypeMirror specializationTargetType;
181-
TypeMirror expectedType = instruction.isQuickening() ? specializedType : genericType;
182-
183-
if (instruction.isQuickening()) {
184-
if (instruction.filteredSpecializations != null) {
185-
specializationTargetType = instruction.getSpecializationSignature().signature().getDynamicOperandTypes().get(operandIndex);
186-
} else {
187-
specializationTargetType = specializedType;
196+
return buildOperandExecution(b, frameState, frame, targetType, operandIndex, operandCount);
197+
}
198+
operandIndex -= instruction.signature.dynamicOperandCount;
199+
200+
int constantOperandAfterCount = instruction.signature.constantOperandsAfterCount;
201+
if (operandIndex < constantOperandAfterCount) {
202+
ConstantOperandModel constantOperand = instruction.operation.constantOperands.after().get(operandIndex);
203+
InstructionImmediate imm = instruction.constantOperandImmediates.get(constantOperand);
204+
if (imm == null) {
205+
throw new AssertionError("Could not find an immediate for constant operand " + constantOperand + " on instruction " + instruction);
206+
}
207+
b.tree(rootNode.readConstantImmediate("$bc", "$bci", "$bytecode", imm, constantOperand.type()));
208+
return false;
209+
}
210+
211+
throw new AssertionError("index=" + index + ", signature=" + instruction.signature);
212+
}
213+
214+
private boolean buildOperandExecution(CodeTreeBuilder b, FrameState frameState, String frame, TypeMirror targetType, int operandIndex, int operandCount) throws AssertionError {
215+
final TypeMirror instructionType = instruction.signature.getDynamicOperandType(operandIndex);
216+
String stackIndex = "$sp - " + (operandCount - operandIndex);
217+
218+
// slow paths never need boxing elimination (e.g. in the uncached interpreter)
219+
final boolean fastPath = frameState.getMode().isFastPath();
220+
221+
/*
222+
* We only want boxing elimination in the cached interpreter, when the operand type is
223+
* boxing eliminated and the instruction is a quickening. Without the quickening we cannot
224+
* boxing eliminate as the operands need to be switched as well.
225+
*/
226+
final boolean boxingEliminated = fastPath && model.isBoxingEliminated(instructionType) && instruction.isQuickening();
227+
228+
boolean expectOtherTypes = instruction.getQuickeningRoot().needsChildBciForBoxingElimination(model, operandIndex);
229+
if (boxingEliminated) {
230+
if (!expectOtherTypes) {
231+
// Sanity check for the internal consistency of boxing elimination and
232+
// needsChildBciForBoxingElimination.
233+
throw new AssertionError("expectOtherTypes must be true with boxing elimination");
234+
}
235+
236+
final ImplicitCastData cast;
237+
if (model.isBoxingEliminated(instructionType) && !ElementUtils.typeEquals(instructionType, targetType) && !ElementUtils.isObject(targetType)) {
238+
cast = instruction.nodeData.getTypeSystem().lookupCast(instructionType, targetType);
239+
if (cast == null) {
240+
throw new AssertionError("Instruction type must match the declared type unless for implicit casts: " + instruction + ": " + instructionType + " -> " + targetType);
188241
}
189-
expectedType = specializedType;
190242
} else {
191-
specializationTargetType = genericType;
192-
expectedType = genericType;
243+
cast = null;
193244
}
194245

195-
String stackIndex = "$sp - " + (operandCount - operandIndex);
196-
ImplicitCastData cast = instruction.nodeData.getTypeSystem().lookupCast(expectedType, specializationTargetType);
246+
if (cast != null) {
247+
b.startStaticCall(cast.getMethod());
248+
}
197249

198-
if (instruction.getQuickeningRoot().needsBoxingElimination(model, operandIndex)) {
199-
if (frameState.getMode().isFastPath()) {
200-
b.startStatement();
201-
if (!ElementUtils.typeEquals(expectedType, specializedType)) {
202-
b.startStaticCall(rootNode.lookupExpectMethod(expectedType, specializedType));
203-
}
204-
if (cast != null) {
205-
b.startStaticCall(cast.getMethod());
206-
}
207-
208-
BytecodeRootNodeElement.startExpectFrameUnsafe(b, frame, expectedType);
209-
b.string(stackIndex);
210-
b.end();
250+
BytecodeRootNodeElement.startExpectFrameUnsafe(b, frame, instructionType);
251+
b.string(stackIndex);
252+
b.end();
211253

212-
if (cast != null) {
213-
b.end();
214-
}
254+
if (cast != null) {
255+
b.end();
256+
}
215257

216-
if (!ElementUtils.typeEquals(expectedType, specializedType)) {
217-
b.end();
218-
}
258+
return true; // unexpected exception thrown
259+
} else {
260+
// expected type is always Object without boxing elimination
261+
262+
/*
263+
* This currently only happens for variadic arguments where the target type is Object[]
264+
* but the expected type is Object. If we introduce new types, we might need to expand
265+
* this check.
266+
*/
267+
if (ElementUtils.typeEquals(targetType, context.getType(Object[].class))) {
268+
b.cast(targetType);
269+
}
270+
271+
if (expectOtherTypes) {
272+
if (fastPath) {
273+
// frame.expectObject(index)
274+
BytecodeRootNodeElement.startExpectFrameUnsafe(b, frame, context.getType(Object.class));
275+
b.string(stackIndex);
219276
b.end();
220-
return true;
277+
return true; // unexpected exception thrown
221278
} else {
222-
if (!ElementUtils.isObject(genericType)) {
223-
b.cast(specializedType);
224-
}
279+
// frame.getValue(index)
225280
BytecodeRootNodeElement.startGetFrameUnsafe(b, frame, null);
226281
b.string(stackIndex);
227282
b.end();
228283
return false;
229284
}
230285
} else {
231-
if (!ElementUtils.isObject(genericType)) {
232-
b.cast(expectedType);
233-
}
286+
/*
287+
* If boxing elimination is not used in the interpreter version then we can assume
288+
* the frame tag is object.
289+
*/
290+
// frame.getObject(index)
234291
b.string(BytecodeRootNodeElement.uncheckedGetFrameObject(frame, stackIndex));
235-
return false;
292+
return false; // no unexpected exception thrown
236293
}
237294
}
238-
operandIndex -= instruction.signature.dynamicOperandCount;
239-
240-
int constantOperandAfterCount = instruction.signature.constantOperandsAfterCount;
241-
if (operandIndex < constantOperandAfterCount) {
242-
ConstantOperandModel constantOperand = instruction.operation.constantOperands.after().get(operandIndex);
243-
InstructionImmediate imm = instruction.constantOperandImmediates.get(constantOperand);
244-
if (imm == null) {
245-
throw new AssertionError("Could not find an immediate for constant operand " + constantOperand + " on instruction " + instruction);
246-
}
247-
b.tree(rootNode.readConstantImmediate("$bc", "$bci", "$bytecode", imm, constantOperand.type()));
248-
return false;
249-
}
250-
251-
throw new AssertionError("index=" + specializationIndex + ", signature=" + instruction.signature);
252295
}
253296

254297
public CodeExecutableElement getQuickenMethod() {
@@ -334,7 +377,7 @@ private CodeExecutableElement createQuickenMethod(FlatNodeGenFactory factory, Fr
334377

335378
for (InstructionModel quickening : relevantQuickenings) {
336379
for (int index = 0; index < quickening.signature.dynamicOperandCount; index++) {
337-
if (model.isBoxingEliminated(quickening.signature.getSpecializedType(index))) {
380+
if (model.isBoxingEliminated(quickening.signature.getDynamicOperandType(index))) {
338381
boxingEliminated.add(index);
339382
}
340383
}
@@ -387,7 +430,7 @@ private CodeExecutableElement createQuickenMethod(FlatNodeGenFactory factory, Fr
387430
List<TypeMirror> dynamicOperandTypes = specializationSignature.signature().getDynamicOperandTypes();
388431

389432
for (int valueIndex : boxingEliminated) {
390-
TypeMirror specializedType = quickening.signature.getSpecializedType(valueIndex);
433+
TypeMirror specializedType = quickening.signature.getDynamicOperandType(valueIndex);
391434
TypeMirror specializationTargetType = dynamicOperandTypes.get(valueIndex);
392435
CodeTree check = factory.createIsImplicitTypeStateCheck(frameState, specializedType, specializationTargetType,
393436
valueIndex + specializationSignature.signature().constantOperandsBeforeCount);
@@ -401,7 +444,7 @@ private CodeExecutableElement createQuickenMethod(FlatNodeGenFactory factory, Fr
401444
}
402445

403446
for (int valueIndex : boxingEliminated) {
404-
TypeMirror specializedType = quickening.signature.getSpecializedType(valueIndex);
447+
TypeMirror specializedType = quickening.signature.getDynamicOperandType(valueIndex);
405448
if (model.isBoxingEliminated(specializedType)) {
406449
b.newLine().string(" ", sep, "(");
407450
b.string("newOperand" + valueIndex);
@@ -414,7 +457,7 @@ private CodeExecutableElement createQuickenMethod(FlatNodeGenFactory factory, Fr
414457
b.end().startBlock();
415458

416459
for (int valueIndex : boxingEliminated) {
417-
TypeMirror specializedType = quickening.signature.getSpecializedType(valueIndex);
460+
TypeMirror specializedType = quickening.signature.getDynamicOperandType(valueIndex);
418461
if (!model.isBoxingEliminated(specializedType)) {
419462
b.startStatement();
420463
b.string("newOperand" + valueIndex, " = undoQuickening(oldOperand" + valueIndex + ")");

truffle/src/com.oracle.truffle.dsl.processor/src/com/oracle/truffle/dsl/processor/bytecode/generator/BytecodeRootNodeElement.java

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6565,7 +6565,7 @@ private CodeExecutableElement createAfterChild() {
65656565
int immediateIndex = 0;
65666566
boolean elseIf = false;
65676567
for (int valueIndex = 0; valueIndex < op.instruction.signature.dynamicOperandCount; valueIndex++) {
6568-
if (op.instruction.needsBoxingElimination(model, valueIndex)) {
6568+
if (op.instruction.needsChildBciForBoxingElimination(model, valueIndex)) {
65696569
elseIf = b.startIf(elseIf);
65706570
b.string("childIndex == " + valueIndex).end().startBlock();
65716571

@@ -17398,7 +17398,7 @@ private CodeExecutableElement lookupTagLeave(InstructionModel instr) {
1739817398
}
1739917399

1740017400
CodeTreeBuilder b = method.createBuilder();
17401-
TypeMirror inputType = instr.specializedType == null ? instr.signature.getSpecializedType(0) : instr.specializedType;
17401+
TypeMirror inputType = instr.specializedType == null ? instr.signature.getDynamicOperandType(0) : instr.specializedType;
1740217402
TypeMirror returnType = instr.signature.returnType;
1740317403

1740417404
boolean isSpecialized = instr.specializedType != null;
@@ -17540,7 +17540,7 @@ private CodeExecutableElement lookupDoPop(InstructionModel instr) {
1754017540
}
1754117541

1754217542
CodeTreeBuilder b = method.createBuilder();
17543-
TypeMirror inputType = instr.signature.getSpecializedType(0);
17543+
TypeMirror inputType = instr.signature.getDynamicOperandType(0);
1754417544

1754517545
boolean isGeneric = ElementUtils.isObject(inputType);
1754617546

@@ -17665,7 +17665,7 @@ private CodeExecutableElement lookupDoBranch(InstructionModel instr) {
1766517665
}
1766617666

1766717667
CodeTreeBuilder b = method.createBuilder();
17668-
TypeMirror inputType = instr.signature.getSpecializedType(0);
17668+
TypeMirror inputType = instr.signature.getDynamicOperandType(0);
1766917669

1767017670
b.startTryBlock();
1767117671
b.startDeclaration(type(boolean.class), "result");
@@ -17729,7 +17729,7 @@ private CodeExecutableElement lookupDoSpecializeBranch(InstructionModel instr) {
1772917729
InstructionModel boxedInstruction = null;
1773017730
InstructionModel unboxedInstruction = null;
1773117731
for (InstructionModel quickening : instr.getFlattenedQuickenedInstructions()) {
17732-
if (ElementUtils.isObject(quickening.signature.getSpecializedType(0))) {
17732+
if (ElementUtils.isObject(quickening.signature.getDynamicOperandType(0))) {
1773317733
boxedInstruction = quickening;
1773417734
} else {
1773517735
unboxedInstruction = quickening;
@@ -18016,7 +18016,7 @@ private CodeExecutableElement lookupDoMergeConditional(InstructionModel instr) {
1801618016
method.getParameters().add(0, new CodeVariableElement(abstractBytecodeNode.asType(), "$this"));
1801718017
}
1801818018

18019-
final TypeMirror inputType = instr.signature.getSpecializedType(1);
18019+
final TypeMirror inputType = instr.signature.getDynamicOperandType(1);
1802018020
final TypeMirror returnType = instr.signature.returnType;
1802118021

1802218022
CodeTreeBuilder b = method.createBuilder();
@@ -18214,7 +18214,7 @@ private CodeExecutableElement lookupDoStoreLocal(InstructionModel instr) {
1821418214
method.getParameters().add(0, new CodeVariableElement(types.Frame, "stackFrame"));
1821518215
}
1821618216

18217-
final TypeMirror inputType = instr.signature.getSpecializedType(0);
18217+
final TypeMirror inputType = instr.signature.getDynamicOperandType(0);
1821818218
final TypeMirror slotType = instr.specializedType != null ? instr.specializedType : type(Object.class);
1821918219

1822018220
CodeTreeBuilder b = method.createBuilder();
@@ -18966,7 +18966,7 @@ private void buildCallExecute(CodeTreeBuilder b, InstructionModel instr, String
1896618966
}
1896718967

1896818968
for (int i = 0; i < instr.signature.dynamicOperandCount; i++) {
18969-
TypeMirror targetType = instr.signature.getGenericType(i);
18969+
TypeMirror targetType = instr.signature.getDynamicOperandType(i);
1897018970
b.startGroup();
1897118971
if (!ElementUtils.isObject(targetType)) {
1897218972
b.cast(targetType);

0 commit comments

Comments
 (0)