Skip to content

Commit

Permalink
feat(avm)!: make JUMP(I) 16-bit (#8443)
Browse files Browse the repository at this point in the history
Earns ~2-5%.

I did not add an 8bit version because the jump currently rarely fits, and once we move to byte-indexed PC, it will almost never fit.
  • Loading branch information
fcarreiro authored Sep 9, 2024
1 parent dc43306 commit 5bb38b1
Show file tree
Hide file tree
Showing 14 changed files with 101 additions and 104 deletions.
6 changes: 6 additions & 0 deletions avm-transpiler/src/bit_traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,12 @@ impl BitsQueryable for u128 {
}
}

impl BitsQueryable for usize {
fn num_bits(&self) -> usize {
get_msb(*self as u128)
}
}

pub fn bits_needed_for<T: BitsQueryable>(val: &T) -> usize {
let num_bits = val.num_bits();
if num_bits < 8 {
Expand Down
8 changes: 4 additions & 4 deletions avm-transpiler/src/opcodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ pub enum AvmOpcode {
L2GASLEFT,
DAGASLEFT,
// Control flow
JUMP,
JUMPI,
JUMP_16,
JUMPI_16,
INTERNALCALL,
INTERNALRETURN,
// Memory
Expand Down Expand Up @@ -129,8 +129,8 @@ impl AvmOpcode {
AvmOpcode::L2GASLEFT => "L2GASLEFT",
AvmOpcode::DAGASLEFT => "DAGASLEFT",
// Machine State - Internal Control Flow
AvmOpcode::JUMP => "JUMP",
AvmOpcode::JUMPI => "JUMPI",
AvmOpcode::JUMP_16 => "JUMP_16",
AvmOpcode::JUMPI_16 => "JUMPI_16",
AvmOpcode::INTERNALCALL => "INTERNALCALL",
AvmOpcode::INTERNALRETURN => "INTERNALRETURN",
// Machine State - Memory
Expand Down
11 changes: 4 additions & 7 deletions avm-transpiler/src/transpile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,20 +101,17 @@ pub fn brillig_to_avm(
BrilligOpcode::Jump { location } => {
let avm_loc = brillig_pcs_to_avm_pcs[*location];
avm_instrs.push(AvmInstruction {
opcode: AvmOpcode::JUMP,
operands: vec![AvmOperand::U32 { value: avm_loc as u32 }],
opcode: AvmOpcode::JUMP_16,
operands: vec![make_operand(16, &avm_loc)],
..Default::default()
});
}
BrilligOpcode::JumpIf { condition, location } => {
let avm_loc = brillig_pcs_to_avm_pcs[*location];
avm_instrs.push(AvmInstruction {
opcode: AvmOpcode::JUMPI,
opcode: AvmOpcode::JUMPI_16,
indirect: Some(ALL_DIRECT),
operands: vec![
AvmOperand::U32 { value: avm_loc as u32 },
AvmOperand::U32 { value: condition.to_usize() as u32 },
],
operands: vec![make_operand(16, &avm_loc), make_operand(16, &condition.0)],
..Default::default()
});
}
Expand Down
114 changes: 54 additions & 60 deletions barretenberg/cpp/src/barretenberg/vm/avm/tests/execution.test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,6 @@ TEST_F(AvmExecutionTests, nestedInternalCalls)
// 0 1 2 3 4
TEST_F(AvmExecutionTests, jumpAndCalldatacopy)
{
GTEST_SKIP();
std::string bytecode_hex = to_hex(OpCode::SET_8) + // opcode SET
"00" // Indirect flag
"03" // U32
Expand All @@ -419,8 +418,8 @@ TEST_F(AvmExecutionTests, jumpAndCalldatacopy)
"00000000" // cd_offset
"00000001" // copy_size
"0000000A" // dst_offset // M[10] = 13, M[11] = 156
+ to_hex(OpCode::JUMP) + // opcode JUMP
"00000005" // jmp_dest (FDIV located at 3)
+ to_hex(OpCode::JUMP_16) + // opcode JUMP
"0005" // jmp_dest (FDIV located at 3)
+ to_hex(OpCode::SUB) + // opcode SUB
"00" // Indirect flag
"06" // FF
Expand Down Expand Up @@ -456,14 +455,16 @@ TEST_F(AvmExecutionTests, jumpAndCalldatacopy)

// JUMP
EXPECT_THAT(instructions.at(3),
AllOf(Field(&Instruction::op_code, OpCode::JUMP),
Field(&Instruction::operands, ElementsAre(VariantWith<uint32_t>(3)))));
AllOf(Field(&Instruction::op_code, OpCode::JUMP_16),
Field(&Instruction::operands, ElementsAre(VariantWith<uint16_t>(5)))));

std::vector<FF> returndata;
auto trace = Execution::gen_trace(instructions, returndata, std::vector<FF>{ 13, 156 }, public_inputs_vec);

// Expected sequence of PCs during execution
std::vector<FF> pc_sequence{ 0, 1, 3, 4 };
std::vector<FF> pc_sequence{
0, 1, 2, 3, 4, 6,
};

for (size_t i = 0; i < 4; i++) {
EXPECT_EQ(trace.at(i + 1).main_pc, pc_sequence.at(i));
Expand Down Expand Up @@ -492,61 +493,70 @@ TEST_F(AvmExecutionTests, jumpAndCalldatacopy)
// We test this bytecode with two calldatacopy values: 9873123 and 0.
TEST_F(AvmExecutionTests, jumpiAndCalldatacopy)
{
GTEST_SKIP();
std::string bytecode_hex = to_hex(OpCode::CALLDATACOPY) + // opcode CALLDATACOPY (no in tag)
"00" // Indirect flag
"00000000" // cd_offset
"00000001" // copy_size
"0000000A" // dst_offset 10
+ to_hex(OpCode::SET_16) + // opcode SET
"00" // Indirect flag
"02" // U16
"0014" // val 20
"0065" // dst_offset 101
+ to_hex(OpCode::JUMPI) + // opcode JUMPI
"00" // Indirect flag
"00000004" // jmp_dest (MUL located at 4)
"0000000A" // cond_offset 10
+ to_hex(OpCode::ADD) + // opcode ADD
"00" // Indirect flag
"02" // U16
"00000065" // addr 101
"00000065" // addr 101
"00000065" // output addr 101
+ to_hex(OpCode::MUL) + // opcode MUL
"00" // Indirect flag
"02" // U16
"00000065" // addr 101
"00000065" // addr 101
"00000066" // output of MUL addr 102
+ to_hex(OpCode::RETURN) + // opcode RETURN
"00" // Indirect flag
"00000000" // ret offset 0
"00000000" // ret size 0
std::string bytecode_hex = to_hex(OpCode::SET_8) + // opcode SET
"00" // Indirect flag
"03" // U32
"00" // val
"00" // dst_offset
+ to_hex(OpCode::SET_8) + // opcode SET
"00" // Indirect flag
"03" // U32
"01" // val
"01" // dst_offset
+ to_hex(OpCode::CALLDATACOPY) + // opcode CALLDATACOPY (no in tag)
"00" // Indirect flag
"00000000" // cd_offset
"00000001" // copy_size
"0000000A" // dst_offset 10
+ to_hex(OpCode::SET_8) + // opcode SET
"00" // Indirect flag
"02" // U16
"14" // val 20
"65" // dst_offset 101
+ to_hex(OpCode::JUMPI_16) + // opcode JUMPI
"00" // Indirect flag
"0006" // jmp_dest (MUL located at 6)
"000A" // cond_offset 10
+ to_hex(OpCode::ADD) + // opcode ADD
"00" // Indirect flag
"02" // U16
"00000065" // addr 101
"00000065" // addr 101
"00000065" // output addr 101
+ to_hex(OpCode::MUL) + // opcode MUL
"00" // Indirect flag
"02" // U16
"00000065" // addr 101
"00000065" // addr 101
"00000066" // output of MUL addr 102
+ to_hex(OpCode::RETURN) + // opcode RETURN
"00" // Indirect flag
"00000000" // ret offset 0
"00000000" // ret size 0
;

auto bytecode = hex_to_bytes(bytecode_hex);
auto instructions = Deserialization::parse(bytecode);

ASSERT_THAT(instructions, SizeIs(6));
ASSERT_THAT(instructions, SizeIs(8));

// We test parsing of JUMPI.

// JUMPI
EXPECT_THAT(
instructions.at(2),
AllOf(Field(&Instruction::op_code, OpCode::JUMPI),
instructions.at(4),
AllOf(Field(&Instruction::op_code, OpCode::JUMPI_16),
Field(&Instruction::operands,
ElementsAre(VariantWith<uint8_t>(0), VariantWith<uint32_t>(4), VariantWith<uint32_t>(10)))));
ElementsAre(VariantWith<uint8_t>(0), VariantWith<uint16_t>(6), VariantWith<uint16_t>(10)))));

std::vector<FF> returndata{};
std::vector<FF> returndata;
auto trace_jump = Execution::gen_trace(instructions, returndata, std::vector<FF>{ 9873123 }, public_inputs_vec);
auto trace_no_jump = Execution::gen_trace(instructions, returndata, std::vector<FF>{ 0 }, public_inputs_vec);

// Expected sequence of PCs during execution with jump
std::vector<FF> pc_sequence_jump{ 0, 1, 2, 4, 5 };
std::vector<FF> pc_sequence_jump{ 0, 1, 2, 3, 4, 6, 7 };
// Expected sequence of PCs during execution without jump
std::vector<FF> pc_sequence_no_jump{ 0, 1, 2, 3, 4, 5 };
std::vector<FF> pc_sequence_no_jump{ 0, 1, 2, 3, 4, 5, 6, 7 };

for (size_t i = 0; i < 5; i++) {
EXPECT_EQ(trace_jump.at(i + 1).main_pc, pc_sequence_jump.at(i));
Expand All @@ -556,22 +566,6 @@ TEST_F(AvmExecutionTests, jumpiAndCalldatacopy)
EXPECT_EQ(trace_no_jump.at(i + 1).main_pc, pc_sequence_no_jump.at(i));
}

// JUMP CASE
// Find the first row enabling the MUL opcode
auto row = std::ranges::find_if(trace_jump.begin(), trace_jump.end(), [](Row r) { return r.main_sel_op_mul == 1; });
EXPECT_EQ(row->main_ic, 400); // 400 = 20 * 20

// Find the first row enabling the addition selector.
row = std::ranges::find_if(trace_jump.begin(), trace_jump.end(), [](Row r) { return r.main_sel_op_add == 1; });
// It must have failed as addition was "jumped over".
EXPECT_EQ(row, trace_jump.end());

// NO JUMP CASE
// Find the first row enabling the MUL opcode
row =
std::ranges::find_if(trace_no_jump.begin(), trace_no_jump.end(), [](Row r) { return r.main_sel_op_mul == 1; });
EXPECT_EQ(row->main_ic, 1600); // 800 = (20 + 20) * (20 + 20)

// traces validation
validate_trace(std::move(trace_jump), public_inputs, { 9873123 });
validate_trace(std::move(trace_no_jump), public_inputs, { 0 });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ const std::unordered_map<OpCode, std::vector<OperandType>> OPCODE_WIRE_FORMAT =
{ OpCode::DAGASLEFT, getter_format },

// Machine State - Internal Control Flow
{ OpCode::JUMP, { OperandType::UINT32 } },
{ OpCode::JUMPI, { OperandType::INDIRECT, OperandType::UINT32, OperandType::UINT32 } },
{ OpCode::JUMP_16, { OperandType::UINT16 } },
{ OpCode::JUMPI_16, { OperandType::INDIRECT, OperandType::UINT16, OperandType::UINT16 } },
{ OpCode::INTERNALCALL, { OperandType::UINT32 } },
{ OpCode::INTERNALRETURN, {} },

Expand Down
10 changes: 5 additions & 5 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -572,13 +572,13 @@ std::vector<Row> Execution::gen_trace(std::vector<Instruction> const& instructio
break;

// Machine State - Internal Control Flow
case OpCode::JUMP:
trace_builder.op_jump(std::get<uint32_t>(inst.operands.at(0)));
case OpCode::JUMP_16:
trace_builder.op_jump(std::get<uint16_t>(inst.operands.at(0)));
break;
case OpCode::JUMPI:
case OpCode::JUMPI_16:
trace_builder.op_jumpi(std::get<uint8_t>(inst.operands.at(0)),
std::get<uint32_t>(inst.operands.at(1)),
std::get<uint32_t>(inst.operands.at(2)));
std::get<uint16_t>(inst.operands.at(1)),
std::get<uint16_t>(inst.operands.at(2)));
break;
case OpCode::INTERNALCALL:
trace_builder.op_internal_call(std::get<uint32_t>(inst.operands.at(0)));
Expand Down
4 changes: 2 additions & 2 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/fixed_gas.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ const std::unordered_map<OpCode, FixedGasTable::GasRow> GAS_COST_TABLE = {
{ OpCode::CALLDATACOPY, make_cost(AVM_CALLDATACOPY_BASE_L2_GAS, 0, AVM_CALLDATACOPY_DYN_L2_GAS, 0) },
{ OpCode::L2GASLEFT, make_cost(AVM_L2GASLEFT_BASE_L2_GAS, 0, AVM_L2GASLEFT_DYN_L2_GAS, 0) },
{ OpCode::DAGASLEFT, make_cost(AVM_DAGASLEFT_BASE_L2_GAS, 0, AVM_DAGASLEFT_DYN_L2_GAS, 0) },
{ OpCode::JUMP, make_cost(AVM_JUMP_BASE_L2_GAS, 0, AVM_JUMP_DYN_L2_GAS, 0) },
{ OpCode::JUMPI, make_cost(AVM_JUMPI_BASE_L2_GAS, 0, AVM_JUMPI_DYN_L2_GAS, 0) },
{ OpCode::JUMP_16, make_cost(AVM_JUMP_BASE_L2_GAS, 0, AVM_JUMP_DYN_L2_GAS, 0) },
{ OpCode::JUMPI_16, make_cost(AVM_JUMPI_BASE_L2_GAS, 0, AVM_JUMPI_DYN_L2_GAS, 0) },
{ OpCode::INTERNALCALL, make_cost(AVM_INTERNALCALL_BASE_L2_GAS, 0, AVM_INTERNALCALL_DYN_L2_GAS, 0) },
{ OpCode::INTERNALRETURN, make_cost(AVM_INTERNALRETURN_BASE_L2_GAS, 0, AVM_INTERNALRETURN_DYN_L2_GAS, 0) },
{ OpCode::SET_8, make_cost(AVM_SET_BASE_L2_GAS, 0, AVM_SET_DYN_L2_GAS, 0) },
Expand Down
8 changes: 4 additions & 4 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/opcode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,10 @@ std::string to_string(OpCode opcode)
case OpCode::DAGASLEFT:
return "DAGASLEFT";
// Machine State - Internal Control Flow
case OpCode::JUMP:
return "JUMP";
case OpCode::JUMPI:
return "JUMPI";
case OpCode::JUMP_16:
return "JUMP_16";
case OpCode::JUMPI_16:
return "JUMPI_16";
case OpCode::INTERNALCALL:
return "INTERNALCALL";
case OpCode::INTERNALRETURN:
Expand Down
4 changes: 2 additions & 2 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/opcode.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ enum class OpCode : uint8_t {
L2GASLEFT,
DAGASLEFT,
// Machine State - Internal Control Flow
JUMP,
JUMPI,
JUMP_16,
JUMPI_16,
INTERNALCALL,
INTERNALRETURN,
// Machine State - Memory
Expand Down
4 changes: 2 additions & 2 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1563,7 +1563,7 @@ void AvmTraceBuilder::op_jump(uint32_t jmp_dest)
auto clk = static_cast<uint32_t>(main_trace.size()) + 1;

// Constrain gas cost
gas_trace_builder.constrain_gas(clk, OpCode::JUMP);
gas_trace_builder.constrain_gas(clk, OpCode::JUMP_16);

main_trace.push_back(Row{
.main_clk = clk,
Expand Down Expand Up @@ -1612,7 +1612,7 @@ void AvmTraceBuilder::op_jumpi(uint8_t indirect, uint32_t jmp_dest, uint32_t con
uint32_t next_pc = !id_zero ? jmp_dest : pc + 1;

// Constrain gas cost
gas_trace_builder.constrain_gas(clk, OpCode::JUMPI);
gas_trace_builder.constrain_gas(clk, OpCode::JUMPI_16);

main_trace.push_back(Row{
.main_clk = clk,
Expand Down
8 changes: 4 additions & 4 deletions yarn-project/simulator/src/avm/avm_gas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ const BaseGasCosts: Record<Opcode, Gas> = {
[Opcode.CALLDATACOPY]: makeCost(c.AVM_CALLDATACOPY_BASE_L2_GAS, 0),
[Opcode.L2GASLEFT]: makeCost(c.AVM_L2GASLEFT_BASE_L2_GAS, 0),
[Opcode.DAGASLEFT]: makeCost(c.AVM_DAGASLEFT_BASE_L2_GAS, 0),
[Opcode.JUMP]: makeCost(c.AVM_JUMP_BASE_L2_GAS, 0),
[Opcode.JUMPI]: makeCost(c.AVM_JUMPI_BASE_L2_GAS, 0),
[Opcode.JUMP_16]: makeCost(c.AVM_JUMP_BASE_L2_GAS, 0),
[Opcode.JUMPI_16]: makeCost(c.AVM_JUMPI_BASE_L2_GAS, 0),
[Opcode.INTERNALCALL]: makeCost(c.AVM_INTERNALCALL_BASE_L2_GAS, 0),
[Opcode.INTERNALRETURN]: makeCost(c.AVM_INTERNALRETURN_BASE_L2_GAS, 0),
[Opcode.SET_8]: makeCost(c.AVM_SET_BASE_L2_GAS, 0),
Expand Down Expand Up @@ -157,8 +157,8 @@ const DynamicGasCosts: Record<Opcode, Gas> = {
[Opcode.CALLDATACOPY]: makeCost(c.AVM_CALLDATACOPY_DYN_L2_GAS, 0),
[Opcode.L2GASLEFT]: makeCost(c.AVM_L2GASLEFT_DYN_L2_GAS, 0),
[Opcode.DAGASLEFT]: makeCost(c.AVM_DAGASLEFT_DYN_L2_GAS, 0),
[Opcode.JUMP]: makeCost(c.AVM_JUMP_DYN_L2_GAS, 0),
[Opcode.JUMPI]: makeCost(c.AVM_JUMPI_DYN_L2_GAS, 0),
[Opcode.JUMP_16]: makeCost(c.AVM_JUMP_DYN_L2_GAS, 0),
[Opcode.JUMPI_16]: makeCost(c.AVM_JUMPI_DYN_L2_GAS, 0),
[Opcode.INTERNALCALL]: makeCost(c.AVM_INTERNALCALL_DYN_L2_GAS, 0),
[Opcode.INTERNALRETURN]: makeCost(c.AVM_INTERNALRETURN_DYN_L2_GAS, 0),
[Opcode.SET_8]: makeCost(c.AVM_SET_DYN_L2_GAS, 0),
Expand Down
10 changes: 5 additions & 5 deletions yarn-project/simulator/src/avm/opcodes/control_flow.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ describe('Control Flow Opcodes', () => {
it('Should (de)serialize correctly', () => {
const buf = Buffer.from([
Jump.opcode, // opcode
...Buffer.from('12345678', 'hex'), // loc
...Buffer.from('1234', 'hex'), // loc
]);
const inst = new Jump(/*loc=*/ 0x12345678);
const inst = new Jump(/*loc=*/ 0x1234);

expect(Jump.deserialize(buf)).toEqual(inst);
expect(inst.serialize()).toEqual(buf);
Expand All @@ -39,10 +39,10 @@ describe('Control Flow Opcodes', () => {
const buf = Buffer.from([
JumpI.opcode, // opcode
0x01, // indirect
...Buffer.from('12345678', 'hex'), // loc
...Buffer.from('a2345678', 'hex'), // condOffset
...Buffer.from('1234', 'hex'), // loc
...Buffer.from('a234', 'hex'), // condOffset
]);
const inst = new JumpI(/*indirect=*/ 1, /*loc=*/ 0x12345678, /*condOffset=*/ 0xa2345678);
const inst = new JumpI(/*indirect=*/ 1, /*loc=*/ 0x1234, /*condOffset=*/ 0xa234);

expect(JumpI.deserialize(buf)).toEqual(inst);
expect(inst.serialize()).toEqual(buf);
Expand Down
Loading

0 comments on commit 5bb38b1

Please sign in to comment.