Skip to content

feat(avm)!: make JUMP(I) 16-bit #8443

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

Merged
fcarreiro merged 1 commit into from
Sep 9, 2024
Merged

feat(avm)!: make JUMP(I) 16-bit #8443

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
6 changes: 6 additions & 0 deletions avm-transpiler/src/bit_traits.rs
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
4 changes: 2 additions & 2 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.cpp
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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