Skip to content

Add fused SELECT_BOOL_REG to endpoints VM #1141

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mtdowling wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add fused SELECT_BOOL_REG to endpoints VM #1141

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions rulesengine/src/main/java/software/amazon/smithy/java/rulesengine/BytecodeCompiler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,19 @@ public Void visitLibraryFunction(FunctionDefinition fn, List<Expression> args) {
throw new RulesEvaluationError("ite requires exactly 3 arguments, got " + args.size());
}

// Optimize: ite(boolRegister, constA, constB) -> SELECT_BOOL_REG
if (args.get(0) instanceof Reference ref
&& args.get(1) instanceof Literal litTrue
&& args.get(2) instanceof Literal litFalse) {
var trueVal = litTrue.toNode().toNode();
var falseVal = litFalse.toNode().toNode();
writer.writeByte(Opcodes.SELECT_BOOL_REG);
writer.writeByte(registerAllocator.getRegister(ref.getName().toString()));
writer.writeShort(writer.getConstantIndex(EndpointUtils.convertNode(trueVal, true)));
writer.writeShort(writer.getConstantIndex(EndpointUtils.convertNode(falseVal, true)));
return null;
}

// Compile condition
compileExpression(args.get(0));

Expand Down
23 changes: 21 additions & 2 deletions rulesengine/src/main/java/software/amazon/smithy/java/rulesengine/BytecodeDisassembler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,8 @@ final class BytecodeDisassembler {
Map.entry(Opcodes.JMP_IF_FALSE, new InstructionDef("JMP_IF_FALSE", Show.JUMP_OFFSET)),
Map.entry(Opcodes.JUMP, new InstructionDef("JUMP", Show.JUMP_OFFSET)),
Map.entry(Opcodes.SUBSTRING_EQ, new InstructionDef("SUBSTRING_EQ", Show.SUBSTRING_EQ)),
Map.entry(Opcodes.SPLIT_GET, new InstructionDef("SPLIT_GET", Show.SPLIT_GET)));
Map.entry(Opcodes.SPLIT_GET, new InstructionDef("SPLIT_GET", Show.SPLIT_GET)),
Map.entry(Opcodes.SELECT_BOOL_REG, new InstructionDef("SELECT_BOOL_REG", Show.SELECT_BOOL)));

private enum Show {
CONST,
Expand All @@ -99,7 +100,8 @@ private enum Show {
PROPERTY,
ARG_COUNT,
SUBSTRING_EQ,
SPLIT_GET
SPLIT_GET,
SELECT_BOOL
}

private record InstructionDef(String name, Show show) {
Expand Down Expand Up @@ -260,6 +262,7 @@ private void writeInstruction(StringBuilder s, BytecodeWalker walker, String ind
||
opcode == Opcodes.JNN_OR_POP
|| (opcode == Opcodes.GET_PROPERTY_REG && i == 1)
|| (opcode == Opcodes.SELECT_BOOL_REG && i >= 1)
||
(opcode == Opcodes.RESOLVE_TEMPLATE && i == 1)) {
s.append(String.format("%5d", value));
Expand Down Expand Up @@ -380,6 +383,22 @@ private void appendSymbolicInfo(StringBuilder s, BytecodeWalker walker, Show sho
}
s.append(")[").append(index).append("]");
}
case SELECT_BOOL -> {
int regIndex = walker.getOperand(0);
int trueIdx = walker.getOperand(1);
int falseIdx = walker.getOperand(2);
if (regIndex >= 0 && regIndex < bytecode.getRegisterDefinitions().length) {
s.append(bytecode.getRegisterDefinitions()[regIndex].name());
}
s.append(" ? ");
if (trueIdx >= 0 && trueIdx < bytecode.getConstantPoolCount()) {
s.append(formatConstant(bytecode.getConstant(trueIdx)));
}
s.append(" : ");
if (falseIdx >= 0 && falseIdx < bytecode.getConstantPoolCount()) {
s.append(formatConstant(bytecode.getConstant(falseIdx)));
}
}
}
}

Expand Down
10 changes: 10 additions & 0 deletions rulesengine/src/main/java/software/amazon/smithy/java/rulesengine/BytecodeEvaluator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,16 @@ private Object runLoop(byte[] instructions, RulesFunction[] functions, Object[]
var sgDelimiter = (String) constantPool[sgDelimIdx];
push(EndpointUtils.splitGet(sgValue, sgDelimiter, sgIndex));
}
case Opcodes.SELECT_BOOL_REG -> {
var selVal = registers[instructions[pc++] & 0xFF];
int selTrue = ((instructions[pc] & 0xFF) << 8) | (instructions[pc + 1] & 0xFF);
pc += 2;
int selFalse = ((instructions[pc] & 0xFF) << 8) | (instructions[pc + 1] & 0xFF);
pc += 2;
push(selVal != null && selVal != Boolean.FALSE
? constantPool[selTrue]
: constantPool[selFalse]);
}
default -> throw new RulesEvaluationError("Unknown rules engine instruction: " + opcode, pc);
}
}
Expand Down
13 changes: 12 additions & 1 deletion rulesengine/src/main/java/software/amazon/smithy/java/rulesengine/BytecodeWalker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public int getOperandCount() {
case Opcodes.GET_PROPERTY_REG, Opcodes.GET_INDEX_REG, Opcodes.RESOLVE_TEMPLATE,
Opcodes.GET_NEGATIVE_INDEX_REG ->
2;
case Opcodes.SUBSTRING, Opcodes.SPLIT_GET -> 3;
case Opcodes.SUBSTRING, Opcodes.SPLIT_GET, Opcodes.SELECT_BOOL_REG -> 3;
case Opcodes.SUBSTRING_EQ -> 5;
default -> -1;
};
Expand Down Expand Up @@ -206,6 +206,16 @@ public int getOperand(int index) {
return code.get(pc + 4) & 0xFF; // index (will be cast to signed byte by caller)
}
break;

case Opcodes.SELECT_BOOL_REG:
if (index == 0) {
return code.get(pc + 1) & 0xFF; // register
} else if (index == 1) {
return ((code.get(pc + 2) & 0xFF) << 8) | (code.get(pc + 3) & 0xFF); // true const
} else if (index == 2) {
return ((code.get(pc + 4) & 0xFF) << 8) | (code.get(pc + 5) & 0xFF); // false const
}
break;
}

throw new IllegalArgumentException("Invalid operand index " + index + " for opcode " + opcode);
Expand Down Expand Up @@ -242,6 +252,7 @@ public static int getInstructionLength(byte opcode) {
3;
case Opcodes.RESOLVE_TEMPLATE, Opcodes.GET_PROPERTY_REG, Opcodes.SUBSTRING -> 4;
case Opcodes.SPLIT_GET -> 5;
case Opcodes.SELECT_BOOL_REG -> 6;
case Opcodes.SUBSTRING_EQ -> 7;
default -> -1;
};
Expand Down
10 changes: 10 additions & 0 deletions rulesengine/src/main/java/software/amazon/smithy/java/rulesengine/Opcodes.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -487,4 +487,14 @@ private Opcodes() {}
* <p>Index is signed: positive = from start, negative = from end (-1 = last)
*/
public static final byte SPLIT_GET = 49;

/**
* Select one of two constant values based on a boolean register.
* Fuses ite(register, constA, constB) into a single opcode.
*
* <p>Stack: [...] => [..., value]
*
* <p><code>SELECT_BOOL_REG [register:byte] [true-const:short] [false-const:short]</code>
*/
public static final byte SELECT_BOOL_REG = 50;
}
3 changes: 1 addition & 2 deletions rulesengine/src/test/java/software/amazon/smithy/java/rulesengine/BytecodeCompilerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -647,8 +647,7 @@ void testCompileIte() {

Bytecode bytecode = compiler.compile();

assertOpcodePresent(bytecode, Opcodes.JMP_IF_FALSE);
assertOpcodePresent(bytecode, Opcodes.JUMP);
assertOpcodePresent(bytecode, Opcodes.SELECT_BOOL_REG);
}

@Test
Expand Down
Loading