Skip to content

Commit

Permalink
feat(avm)!: variants for REVERT opcode (#8487)
Browse files Browse the repository at this point in the history
3-6% bytecode improvement.
  • Loading branch information
fcarreiro authored Sep 11, 2024
1 parent bce1eea commit a0c8915
Show file tree
Hide file tree
Showing 12 changed files with 65 additions and 27 deletions.
6 changes: 4 additions & 2 deletions avm-transpiler/src/opcodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ pub enum AvmOpcode {
STATICCALL,
DELEGATECALL,
RETURN,
REVERT,
REVERT_8,
REVERT_16,
// Misc
DEBUGLOG,
// Gadgets
Expand Down Expand Up @@ -189,7 +190,8 @@ impl AvmOpcode {
AvmOpcode::STATICCALL => "STATICCALL",
AvmOpcode::DELEGATECALL => "DELEGATECALL",
AvmOpcode::RETURN => "RETURN",
AvmOpcode::REVERT => "REVERT",
AvmOpcode::REVERT_8 => "REVERT_8",
AvmOpcode::REVERT_16 => "REVERT_16",

// Misc
AvmOpcode::DEBUGLOG => "DEBUGLOG",
Expand Down
16 changes: 13 additions & 3 deletions avm-transpiler/src/transpile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -274,12 +274,22 @@ pub fn brillig_to_avm(
});
}
BrilligOpcode::Trap { revert_data } => {
let bits_needed = [revert_data.pointer.0, revert_data.size]
.iter()
.map(bits_needed_for)
.max()
.unwrap();
let avm_opcode = match bits_needed {
8 => AvmOpcode::REVERT_8,
16 => AvmOpcode::REVERT_16,
_ => panic!("REVERT only support 8 or 16 bit encodings, got: {}", bits_needed),
};
avm_instrs.push(AvmInstruction {
opcode: AvmOpcode::REVERT,
opcode: avm_opcode,
indirect: Some(ZEROTH_OPERAND_INDIRECT),
operands: vec![
AvmOperand::U32 { value: revert_data.pointer.0 as u32 },
AvmOperand::U32 { value: revert_data.size as u32 },
make_operand(bits_needed, &revert_data.pointer.0),
make_operand(bits_needed, &revert_data.size),
],
..Default::default()
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,8 @@ const std::unordered_map<OpCode, std::vector<OperandType>> OPCODE_WIRE_FORMAT =
// DELEGATECALL, -- not in simulator
{ OpCode::RETURN, { OperandType::INDIRECT, OperandType::UINT32, OperandType::UINT32 } },
// REVERT,
{ OpCode::REVERT, { OperandType::INDIRECT, OperandType::UINT32, OperandType::UINT32 } },
{ OpCode::REVERT_8, { OperandType::INDIRECT, OperandType::UINT8, OperandType::UINT8 } },
{ OpCode::REVERT_16, { OperandType::INDIRECT, OperandType::UINT16, OperandType::UINT16 } },

// Misc
{ OpCode::DEBUGLOG,
Expand Down
14 changes: 11 additions & 3 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -815,10 +815,18 @@ std::vector<Row> Execution::gen_trace(std::vector<Instruction> const& instructio

break;
}
case OpCode::REVERT: {
case OpCode::REVERT_8: {
auto ret = trace_builder.op_revert(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<uint8_t>(inst.operands.at(1)),
std::get<uint8_t>(inst.operands.at(2)));
returndata.insert(returndata.end(), ret.begin(), ret.end());

break;
}
case OpCode::REVERT_16: {
auto ret = trace_builder.op_revert(std::get<uint8_t>(inst.operands.at(0)),
std::get<uint16_t>(inst.operands.at(1)),
std::get<uint16_t>(inst.operands.at(2)));
returndata.insert(returndata.end(), ret.begin(), ret.end());

break;
Expand Down
3 changes: 2 additions & 1 deletion barretenberg/cpp/src/barretenberg/vm/avm/trace/fixed_gas.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ const std::unordered_map<OpCode, FixedGasTable::GasRow> GAS_COST_TABLE = {
{ OpCode::STATICCALL, make_cost(AVM_STATICCALL_BASE_L2_GAS, 0, AVM_STATICCALL_DYN_L2_GAS, 0) },
{ OpCode::DELEGATECALL, make_cost(AVM_DELEGATECALL_BASE_L2_GAS, 0, AVM_DELEGATECALL_DYN_L2_GAS, 0) },
{ OpCode::RETURN, make_cost(AVM_RETURN_BASE_L2_GAS, 0, AVM_RETURN_DYN_L2_GAS, 0) },
{ OpCode::REVERT, make_cost(AVM_REVERT_BASE_L2_GAS, 0, AVM_REVERT_DYN_L2_GAS, 0) },
{ OpCode::REVERT_8, make_cost(AVM_REVERT_BASE_L2_GAS, 0, AVM_REVERT_DYN_L2_GAS, 0) },
{ OpCode::REVERT_16, make_cost(AVM_REVERT_BASE_L2_GAS, 0, AVM_REVERT_DYN_L2_GAS, 0) },
{ OpCode::DEBUGLOG, make_cost(AVM_DEBUGLOG_BASE_L2_GAS, 0, AVM_DEBUGLOG_DYN_L2_GAS, 0) },
{ OpCode::KECCAK, make_cost(AVM_KECCAK_BASE_L2_GAS, 0, AVM_KECCAK_DYN_L2_GAS, 0) },
{ OpCode::POSEIDON2, make_cost(AVM_POSEIDON2_BASE_L2_GAS, 0, AVM_POSEIDON2_DYN_L2_GAS, 0) },
Expand Down
6 changes: 4 additions & 2 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/opcode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,10 @@ std::string to_string(OpCode opcode)
return "DELEGATECALL";
case OpCode::RETURN:
return "RETURN";
case OpCode::REVERT:
return "REVERT";
case OpCode::REVERT_8:
return "REVERT_8";
case OpCode::REVERT_16:
return "REVERT_16";
// Misc
case OpCode::DEBUGLOG:
return "DEBUGLOG";
Expand Down
3 changes: 2 additions & 1 deletion barretenberg/cpp/src/barretenberg/vm/avm/trace/opcode.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,8 @@ enum class OpCode : uint8_t {
STATICCALL,
DELEGATECALL,
RETURN,
REVERT,
REVERT_8,
REVERT_16,

// Misc
DEBUGLOG,
Expand Down
6 changes: 4 additions & 2 deletions yarn-project/simulator/src/avm/avm_gas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,8 @@ const BaseGasCosts: Record<Opcode, Gas> = {
[Opcode.STATICCALL]: makeCost(c.AVM_STATICCALL_BASE_L2_GAS, 0),
[Opcode.DELEGATECALL]: makeCost(c.AVM_DELEGATECALL_BASE_L2_GAS, 0),
[Opcode.RETURN]: makeCost(c.AVM_RETURN_BASE_L2_GAS, 0),
[Opcode.REVERT]: makeCost(c.AVM_REVERT_BASE_L2_GAS, 0),
[Opcode.REVERT_8]: makeCost(c.AVM_REVERT_BASE_L2_GAS, 0),
[Opcode.REVERT_16]: makeCost(c.AVM_REVERT_BASE_L2_GAS, 0),
[Opcode.DEBUGLOG]: makeCost(c.AVM_DEBUGLOG_BASE_L2_GAS, 0),
[Opcode.KECCAK]: makeCost(c.AVM_KECCAK_BASE_L2_GAS, 0),
[Opcode.POSEIDON2]: makeCost(c.AVM_POSEIDON2_BASE_L2_GAS, 0),
Expand Down Expand Up @@ -210,7 +211,8 @@ const DynamicGasCosts: Record<Opcode, Gas> = {
[Opcode.STATICCALL]: makeCost(c.AVM_STATICCALL_DYN_L2_GAS, 0),
[Opcode.DELEGATECALL]: makeCost(c.AVM_DELEGATECALL_DYN_L2_GAS, 0),
[Opcode.RETURN]: makeCost(c.AVM_RETURN_DYN_L2_GAS, 0),
[Opcode.REVERT]: makeCost(c.AVM_REVERT_DYN_L2_GAS, 0),
[Opcode.REVERT_8]: makeCost(c.AVM_REVERT_DYN_L2_GAS, 0),
[Opcode.REVERT_16]: makeCost(c.AVM_REVERT_DYN_L2_GAS, 0),
[Opcode.DEBUGLOG]: makeCost(c.AVM_DEBUGLOG_DYN_L2_GAS, 0),
[Opcode.KECCAK]: makeCost(c.AVM_KECCAK_DYN_L2_GAS, 0),
[Opcode.POSEIDON2]: makeCost(c.AVM_POSEIDON2_DYN_L2_GAS, 0),
Expand Down
13 changes: 8 additions & 5 deletions yarn-project/simulator/src/avm/opcodes/external_calls.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -287,14 +287,17 @@ describe('External Calls', () => {
describe('REVERT', () => {
it('Should (de)serialize correctly', () => {
const buf = Buffer.from([
Revert.opcode, // opcode
Opcode.REVERT_16, // opcode
0x01, // indirect
...Buffer.from('12345678', 'hex'), // returnOffset
...Buffer.from('a2345678', 'hex'), // retSize
...Buffer.from('1234', 'hex'), // returnOffset
...Buffer.from('a234', 'hex'), // retSize
]);
const inst = new Revert(/*indirect=*/ 0x01, /*returnOffset=*/ 0x12345678, /*retSize=*/ 0xa2345678);
const inst = new Revert(/*indirect=*/ 0x01, /*returnOffset=*/ 0x1234, /*retSize=*/ 0xa234).as(
Opcode.REVERT_16,
Revert.wireFormat16,
);

expect(Revert.deserialize(buf)).toEqual(inst);
expect(Revert.as(Revert.wireFormat16).deserialize(buf)).toEqual(inst);
expect(inst.serialize()).toEqual(buf);
});

Expand Down
16 changes: 11 additions & 5 deletions yarn-project/simulator/src/avm/opcodes/external_calls.ts
Original file line number Diff line number Diff line change
Expand Up @@ -183,13 +183,19 @@ export class Return extends Instruction {

export class Revert extends Instruction {
static type: string = 'REVERT';
static readonly opcode: Opcode = Opcode.REVERT;
// Informs (de)serialization. See Instruction.deserialize.
static readonly wireFormat: OperandType[] = [
static readonly opcode: Opcode = Opcode.REVERT_8;

static readonly wireFormat8: OperandType[] = [
OperandType.UINT8,
OperandType.UINT8,
OperandType.UINT32,
OperandType.UINT32,
OperandType.UINT8,
OperandType.UINT8,
];
static readonly wireFormat16: OperandType[] = [
OperandType.UINT8,
OperandType.UINT8,
OperandType.UINT16,
OperandType.UINT16,
];

constructor(private indirect: number, private returnOffset: number, private retSize: number) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,8 @@ const INSTRUCTION_SET = () =>
[StaticCall.opcode, Instruction.deserialize.bind(StaticCall)],
//[DelegateCall.opcode, Instruction.deserialize.bind(DelegateCall)],
[Return.opcode, Instruction.deserialize.bind(Return)],
[Revert.opcode, Instruction.deserialize.bind(Revert)],
[Opcode.REVERT_8, Revert.as(Revert.wireFormat8).deserialize],
[Opcode.REVERT_16, Revert.as(Revert.wireFormat16).deserialize],

// Misc
[DebugLog.opcode, Instruction.deserialize.bind(DebugLog)],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ export enum Opcode {
STATICCALL,
DELEGATECALL,
RETURN,
REVERT,
REVERT_8,
REVERT_16,
// Misc
DEBUGLOG,
// Gadgets
Expand Down

0 comments on commit a0c8915

Please sign in to comment.