Skip to content

Commit 423ad8b

Browse files
chumerchumer
committed
[GR-65439] Fix two off-by-one errors in the dynamic varargs implementation.
PullRequest: graal/21266
2 parents e6aca1f + 36c69e5 commit 423ad8b

File tree

3 files changed

+302
-7
lines changed

3 files changed

+302
-7
lines changed

‎truffle/src/com.oracle.truffle.api.bytecode.test/src/com/oracle/truffle/api/bytecode/test/basic_interpreter/BasicInterpreter.java

Lines changed: 91 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -909,7 +909,7 @@ public static Bindings doDefault(
909909
@Operation
910910
static final class Variadic0Operation {
911911
@Specialization
912-
public static Object[] variadic(@Variadic Object[] args) {
912+
public static Object[] doDefault(@Variadic Object[] args) {
913913
return args;
914914
}
915915
}
@@ -918,7 +918,7 @@ public static Object[] variadic(@Variadic Object[] args) {
918918
static final class Variadic1Operation {
919919
@Specialization
920920
@SuppressWarnings("unused")
921-
public static Object[] variadic(long arg0, @Variadic Object[] args) {
921+
public static Object[] doDefault(long arg0, @Variadic Object[] args) {
922922
return args;
923923
}
924924
}
@@ -927,7 +927,7 @@ public static Object[] variadic(long arg0, @Variadic Object[] args) {
927927
static final class VariadicOffsetOperation {
928928
@Specialization
929929
@SuppressWarnings("unused")
930-
public static Object[] variadic(@Variadic(startOffset = 4) Object[] args) {
930+
public static Object[] doDefault(@Variadic(startOffset = 4) Object[] args) {
931931
assertTrue(args.length >= 3);
932932
for (int i = 0; i < 4; i++) {
933933
assertNull(args[i]);
@@ -942,7 +942,7 @@ static final class DynamicVariadic {
942942

943943
@Specialization
944944
@SuppressWarnings("unused")
945-
public static Object[] pass(@Variadic Object[] args) {
945+
public static Object[] doDefault(@Variadic Object[] args) {
946946
return args;
947947
}
948948
}
@@ -953,11 +953,97 @@ static final class DynamicVariadicNull {
953953

954954
@Specialization
955955
@SuppressWarnings("unused")
956-
public static Object[] pass() {
956+
public static Object[] doDefault() {
957957
return null;
958958
}
959959
}
960960

961+
@Operation
962+
@Variadic
963+
static final class DynamicVariadicNums {
964+
965+
@Specialization
966+
@SuppressWarnings("unused")
967+
public static Object[] doDefault(long a) {
968+
Object[] res = new Long[(int) a];
969+
for (long i = 0; i < a; i++) {
970+
res[(int) i] = i;
971+
}
972+
return res;
973+
}
974+
}
975+
976+
@Operation
977+
static final class VariadicAddInt {
978+
@Specialization
979+
@SuppressWarnings("unused")
980+
public static long doDefault(long a, @Variadic Object[] args) {
981+
long result = 0;
982+
for (Object arg : args) {
983+
if (arg instanceof Long i) {
984+
result += i * a;
985+
} else {
986+
CompilerDirectives.transferToInterpreterAndInvalidate();
987+
throw new AssertionError("Expected 'arg' to be long, found: " + arg.getClass().getSimpleName());
988+
}
989+
}
990+
return result;
991+
}
992+
}
993+
994+
@Operation
995+
static final class VariadicAddLArr {
996+
@Specialization
997+
@SuppressWarnings("unused")
998+
public static long doDefault(long[] o, @Variadic Object[] args) {
999+
long result = 0;
1000+
for (Object arg : args) {
1001+
if (arg instanceof Long i) {
1002+
result += i;
1003+
} else {
1004+
CompilerDirectives.transferToInterpreterAndInvalidate();
1005+
throw new AssertionError("Expected 'arg' to be long, found: " + arg.getClass().getSimpleName());
1006+
}
1007+
}
1008+
return result;
1009+
}
1010+
}
1011+
1012+
@Operation
1013+
static final class VariadicAddIntLArr {
1014+
@Specialization
1015+
@SuppressWarnings("unused")
1016+
public static long doDefault(long a, long[] o, @Variadic Object[] args) {
1017+
long result = 0;
1018+
for (Object arg : args) {
1019+
if (arg instanceof Long i) {
1020+
result += i * a;
1021+
} else {
1022+
CompilerDirectives.transferToInterpreterAndInvalidate();
1023+
throw new AssertionError("Expected 'arg' to be long, found: " + arg.getClass().getSimpleName());
1024+
}
1025+
}
1026+
return result;
1027+
}
1028+
}
1029+
1030+
@Operation
1031+
static final class VariadicAddIntIntLArrLArr {
1032+
@Specialization
1033+
@SuppressWarnings("unused")
1034+
public static long doDefault(long a, long b, long[] o, long[] p, @Variadic Object[] args) {
1035+
long result = 0;
1036+
for (Object arg : args) {
1037+
if (arg instanceof Long i) {
1038+
result += i * a * b;
1039+
} else {
1040+
CompilerDirectives.transferToInterpreterAndInvalidate();
1041+
throw new AssertionError("Expected 'arg' to be long, found: " + arg.getClass().getSimpleName());
1042+
}
1043+
}
1044+
return result;
1045+
}
1046+
}
9611047
}
9621048

9631049
class TestClosure {

‎truffle/src/com.oracle.truffle.api.bytecode.test/src/com/oracle/truffle/api/bytecode/test/basic_interpreter/VariadicTest.java

Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,211 @@ public void testNullReturn() {
327327
assertEquals("The operation DynamicVariadicNull must return a non-null value, but did return a null value.", e.getMessage());
328328
}
329329

330+
@Test
331+
public void testDynamicVariadicOneScalarOneVariadic() {
332+
var root = parseNode("testDynamicVariadicOneScalarOneVariadic", (b) -> {
333+
b.beginRoot();
334+
b.beginReturn();
335+
336+
b.beginVariadicAddInt();
337+
b.emitLoadConstant(5L);
338+
339+
b.beginDynamicVariadicNums();
340+
b.emitLoadConstant(3L);
341+
b.endDynamicVariadicNums();
342+
b.endVariadicAddInt();
343+
344+
b.endReturn();
345+
b.endRoot();
346+
});
347+
assertEquals(2L * 5L + 1L * 5L, root.getCallTarget().call());
348+
}
349+
350+
@Test
351+
public void testDynamicVariadicOneArrayOneVariadic() {
352+
var root = parseNode("testDynamicVariadicOneArrayOneVariadic", (b) -> {
353+
b.beginRoot();
354+
b.beginReturn();
355+
356+
b.beginVariadicAddLArr();
357+
b.emitLoadConstant(new long[]{1L, 2L, 3L});
358+
359+
b.beginDynamicVariadicNums();
360+
b.emitLoadConstant(3L);
361+
b.endDynamicVariadicNums();
362+
b.endVariadicAddLArr();
363+
364+
b.endReturn();
365+
b.endRoot();
366+
});
367+
assertEquals(2L + 1L, root.getCallTarget().call());
368+
}
369+
370+
@Test
371+
public void testDynamicVariadicOneScalarOneArrayOneVariadic() {
372+
var root = parseNode("testDynamicVariadicOneScalarOneArrayOneVariadic", (b) -> {
373+
b.beginRoot();
374+
b.beginReturn();
375+
376+
b.beginVariadicAddIntLArr();
377+
b.emitLoadConstant(7L);
378+
b.emitLoadConstant(new long[]{1L, 2L, 3L});
379+
380+
b.beginDynamicVariadicNums();
381+
b.emitLoadConstant(3L);
382+
b.endDynamicVariadicNums();
383+
b.endVariadicAddIntLArr();
384+
385+
b.endReturn();
386+
b.endRoot();
387+
});
388+
assertEquals(2L * 7L + 1L * 7L, root.getCallTarget().call());
389+
}
390+
391+
@Test
392+
public void testDynamicVariadicOneScalarMultipleVariadic() {
393+
/**
394+
* Note regarding the test name: 'Multiple variadic' means one @Variadic argument, but
395+
* multiple calls to operations returning @Variadic. 'Multiple scalar' or 'multiple array'
396+
* really means that the operation has multiples of those arguments.
397+
*/
398+
var root = parseNode("testDynamicVariadicOneScalarMultipleVariadic", (b) -> {
399+
b.beginRoot();
400+
b.beginReturn();
401+
402+
b.beginVariadicAddInt();
403+
b.emitLoadConstant(5L);
404+
405+
b.beginDynamicVariadicNums();
406+
b.emitLoadConstant(3L);
407+
b.endDynamicVariadicNums();
408+
409+
b.beginDynamicVariadicNums();
410+
b.emitLoadConstant(3L);
411+
b.endDynamicVariadicNums();
412+
413+
b.beginDynamicVariadicNums();
414+
b.emitLoadConstant(3L);
415+
b.endDynamicVariadicNums();
416+
b.endVariadicAddInt();
417+
418+
b.endReturn();
419+
b.endRoot();
420+
});
421+
assertEquals(2L * 5L + 1L * 5L + 2L * 5L + 1L * 5L + 2L * 5L + 1L * 5L, root.getCallTarget().call());
422+
}
423+
424+
@Test
425+
public void testDynamicVariadicOneArrayMultipleVariadic() {
426+
var root = parseNode("testDynamicVariadicOneArrayMultipleVariadic", (b) -> {
427+
b.beginRoot();
428+
b.beginReturn();
429+
430+
b.beginVariadicAddLArr();
431+
b.emitLoadConstant(new long[]{1L, 2L, 3L});
432+
433+
b.beginDynamicVariadicNums();
434+
b.emitLoadConstant(3L);
435+
b.endDynamicVariadicNums();
436+
437+
b.beginDynamicVariadicNums();
438+
b.emitLoadConstant(3L);
439+
b.endDynamicVariadicNums();
440+
441+
b.beginDynamicVariadicNums();
442+
b.emitLoadConstant(3L);
443+
b.endDynamicVariadicNums();
444+
b.endVariadicAddLArr();
445+
446+
b.endReturn();
447+
b.endRoot();
448+
});
449+
assertEquals(2L + 1L + 2L + 1L + 2L + 1L, root.getCallTarget().call());
450+
}
451+
452+
@Test
453+
public void testDynamicVariadicOneScalarOneArrayMultipleVariadic() {
454+
var root = parseNode("testDynamicVariadicOneScalarOneArrayMultipleVariadic", (b) -> {
455+
b.beginRoot();
456+
b.beginReturn();
457+
458+
b.beginVariadicAddIntLArr();
459+
b.emitLoadConstant(7L);
460+
b.emitLoadConstant(new long[]{1L, 2L, 3L});
461+
462+
b.beginDynamicVariadicNums();
463+
b.emitLoadConstant(3L);
464+
b.endDynamicVariadicNums();
465+
466+
b.beginDynamicVariadicNums();
467+
b.emitLoadConstant(3L);
468+
b.endDynamicVariadicNums();
469+
470+
b.beginDynamicVariadicNums();
471+
b.emitLoadConstant(3L);
472+
b.endDynamicVariadicNums();
473+
b.endVariadicAddIntLArr();
474+
475+
b.endReturn();
476+
b.endRoot();
477+
});
478+
assertEquals(2L * 7L + 1L * 7L + 2L * 7L + 1L * 7L + 2L * 7L + 1L * 7L, root.getCallTarget().call());
479+
}
480+
481+
@Test
482+
public void testDynamicVariadicMultipleScalarMultipleArrayOneVariadic() {
483+
var root = parseNode("testDynamicVariadicMultipleScalarMultipleArrayOneVariadic", (b) -> {
484+
b.beginRoot();
485+
b.beginReturn();
486+
487+
b.beginVariadicAddIntIntLArrLArr();
488+
b.emitLoadConstant(7L);
489+
b.emitLoadConstant(2L);
490+
b.emitLoadConstant(new long[]{1L, 2L, 3L});
491+
b.emitLoadConstant(new long[]{4L, 5L, 6L});
492+
493+
b.beginDynamicVariadicNums();
494+
b.emitLoadConstant(3L);
495+
b.endDynamicVariadicNums();
496+
b.endVariadicAddIntIntLArrLArr();
497+
498+
b.endReturn();
499+
b.endRoot();
500+
});
501+
assertEquals(2L * 7L * 2L + 1L * 7L * 2L, root.getCallTarget().call());
502+
}
503+
504+
@Test
505+
public void testDynamicVariadicMultipleScalarMultipleArrayMultipleVariadic() {
506+
var root = parseNode("testDynamicVariadicMultipleScalarMultipleArrayMultipleVariadic", (b) -> {
507+
b.beginRoot();
508+
b.beginReturn();
509+
510+
b.beginVariadicAddIntIntLArrLArr();
511+
b.emitLoadConstant(7L);
512+
b.emitLoadConstant(2L);
513+
b.emitLoadConstant(new long[]{1L, 2L, 3L});
514+
b.emitLoadConstant(new long[]{4L, 5L, 6L});
515+
516+
b.beginDynamicVariadicNums();
517+
b.emitLoadConstant(3L);
518+
b.endDynamicVariadicNums();
519+
520+
b.beginDynamicVariadicNums();
521+
b.emitLoadConstant(3L);
522+
b.endDynamicVariadicNums();
523+
524+
b.beginDynamicVariadicNums();
525+
b.emitLoadConstant(3L);
526+
b.endDynamicVariadicNums();
527+
b.endVariadicAddIntIntLArrLArr();
528+
529+
b.endReturn();
530+
b.endRoot();
531+
});
532+
assertEquals(2L * 7L * 2L + 1L * 7L * 2L + 2L * 7L * 2L + 1L * 7L * 2L + 2L * 7L * 2L + 1L * 7L * 2L, root.getCallTarget().call());
533+
}
534+
330535
private static int countInstructions(BytecodeRootNode root, String name) throws AssertionError {
331536
int count = 0;
332537
for (Instruction instr : root.getBytecodeNode().getInstructions()) {

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6226,7 +6226,7 @@ private CodeExecutableElement createAfterChild() {
62266226

62276227
if (op.isVariadic && model.hasVariadicReturn) {
62286228
if (op.instruction.signature.dynamicOperandCount > 1) {
6229-
b.startIf().string("childIndex > ").string(op.instruction.signature.dynamicOperandCount - 1).end().startBlock();
6229+
b.startIf().string("childIndex > ").string(op.instruction.signature.dynamicOperandCount - 2).end().startBlock();
62306230
}
62316231

62326232
b.startIf().string("isVariadicReturn(operationCode)").end().startBlock();
@@ -6239,7 +6239,11 @@ private CodeExecutableElement createAfterChild() {
62396239
"variadicReturnIndices.length * 2").end().end();
62406240
b.tree(operationStack.write(op, operationFields.variadicReturnIndices, "variadicReturnIndices"));
62416241
b.end();
6242-
b.statement("variadicReturnIndices[numVariadicReturnIndices] = childIndex");
6242+
if (op.instruction.signature.dynamicOperandCount > 1) {
6243+
b.statement("variadicReturnIndices[numVariadicReturnIndices] = childIndex - " + (op.instruction.signature.dynamicOperandCount - 1));
6244+
} else {
6245+
b.statement("variadicReturnIndices[numVariadicReturnIndices] = childIndex");
6246+
}
62436247
b.tree(operationStack.write(op, operationFields.numVariadicReturnIndices, "numVariadicReturnIndices + 1"));
62446248

62456249
b.end(); // if isVariadicReturn

0 commit comments

Comments
 (0)