Skip to content

Commit dc43306

Browse files
authored
feat(avm)!: variants for SET opcode (#8441)
Saves approx 5-10% bytecode size* This one is not expected to save a lot of space because it already unofficially had variants (however the addresses are getting smaller now). This PR also * Allows SET_FF with size field * Therefore removes extra Brillig codegen necessary to handle big fields * Makes serde of SET opcodes uniform (does not need special casing) * Avoids extra casting in the transpiler, making set opcodes 1-1 with Brillig (no pc adjustment needed) *don't believe the benchmark run, that one is against master and takes into account the whole PR stack.
1 parent 5b27fbc commit dc43306

31 files changed

+732
-794
lines changed

avm-transpiler/src/instructions.rs

+5
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
use std::fmt::{self, Display};
22
use std::fmt::{Debug, Formatter};
33

4+
use acvm::{AcirField, FieldElement};
5+
46
use crate::opcodes::AvmOpcode;
57

68
/// Common values of the indirect instruction flag
@@ -110,6 +112,7 @@ pub enum AvmOperand {
110112
U32 { value: u32 },
111113
U64 { value: u64 },
112114
U128 { value: u128 },
115+
FF { value: FieldElement },
113116
}
114117

115118
impl Display for AvmOperand {
@@ -120,6 +123,7 @@ impl Display for AvmOperand {
120123
AvmOperand::U32 { value } => write!(f, " U32:{}", value),
121124
AvmOperand::U64 { value } => write!(f, " U64:{}", value),
122125
AvmOperand::U128 { value } => write!(f, " U128:{}", value),
126+
AvmOperand::FF { value } => write!(f, " FF:{}", value),
123127
}
124128
}
125129
}
@@ -132,6 +136,7 @@ impl AvmOperand {
132136
AvmOperand::U32 { value } => value.to_be_bytes().to_vec(),
133137
AvmOperand::U64 { value } => value.to_be_bytes().to_vec(),
134138
AvmOperand::U128 { value } => value.to_be_bytes().to_vec(),
139+
AvmOperand::FF { value } => value.to_be_bytes(),
135140
}
136141
}
137142
}

avm-transpiler/src/opcodes.rs

+12-2
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,12 @@ pub enum AvmOpcode {
4141
INTERNALCALL,
4242
INTERNALRETURN,
4343
// Memory
44-
SET,
44+
SET_8,
45+
SET_16,
46+
SET_32,
47+
SET_64,
48+
SET_128,
49+
SET_FF,
4550
MOV_8,
4651
MOV_16,
4752
CMOV,
@@ -129,7 +134,12 @@ impl AvmOpcode {
129134
AvmOpcode::INTERNALCALL => "INTERNALCALL",
130135
AvmOpcode::INTERNALRETURN => "INTERNALRETURN",
131136
// Machine State - Memory
132-
AvmOpcode::SET => "SET",
137+
AvmOpcode::SET_8 => "SET_8",
138+
AvmOpcode::SET_16 => "SET_16",
139+
AvmOpcode::SET_32 => "SET_32",
140+
AvmOpcode::SET_64 => "SET_64",
141+
AvmOpcode::SET_128 => "SET_128",
142+
AvmOpcode::SET_FF => "SET_FF",
133143
AvmOpcode::MOV_8 => "MOV_8",
134144
AvmOpcode::MOV_16 => "MOV_16",
135145
AvmOpcode::CMOV => "CMOV",

avm-transpiler/src/transpile.rs

+21-30
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use acvm::acir::circuit::BrilligOpcodeLocation;
66
use acvm::brillig_vm::brillig::{
77
BinaryFieldOp, BinaryIntOp, BlackBoxOp, HeapArray, HeapVector, MemoryAddress, ValueOrArray,
88
};
9-
use acvm::{AcirField, FieldElement};
9+
use acvm::FieldElement;
1010
use noirc_errors::debug_info::DebugInfo;
1111

1212
use crate::bit_traits::bits_needed_for;
@@ -674,45 +674,38 @@ fn handle_const(
674674
) {
675675
let tag = tag_from_bit_size(*bit_size);
676676
let dest = destination.to_usize() as u32;
677-
678-
if !matches!(tag, AvmTypeTag::FIELD) {
679-
avm_instrs.push(generate_set_instruction(tag, dest, value.to_u128(), indirect));
680-
} else {
681-
// We can't fit a field in an instruction. This should've been handled in Brillig.
682-
let field = value;
683-
if field.num_bits() > 128 {
684-
panic!("SET: Field value doesn't fit in 128 bits, that's not supported!");
685-
}
686-
avm_instrs.extend([
687-
generate_set_instruction(AvmTypeTag::UINT128, dest, field.to_u128(), indirect),
688-
generate_cast_instruction(dest, indirect, dest, indirect, AvmTypeTag::FIELD),
689-
]);
690-
}
677+
avm_instrs.push(generate_set_instruction(tag, dest, value, indirect));
691678
}
692679

693680
/// Generates an AVM SET instruction.
694681
fn generate_set_instruction(
695682
tag: AvmTypeTag,
696683
dest: u32,
697-
value: u128,
684+
value: &FieldElement,
698685
indirect: bool,
699686
) -> AvmInstruction {
687+
let bits_needed_val = bits_needed_for(value);
688+
let bits_needed_mem = if bits_needed_val >= 16 { 16 } else { bits_needed_for(&dest) };
689+
assert!(bits_needed_mem <= 16);
690+
let bits_needed_opcode = bits_needed_val.max(bits_needed_mem);
691+
692+
let set_opcode = match bits_needed_opcode {
693+
8 => AvmOpcode::SET_8,
694+
16 => AvmOpcode::SET_16,
695+
32 => AvmOpcode::SET_32,
696+
64 => AvmOpcode::SET_64,
697+
128 => AvmOpcode::SET_128,
698+
254 => AvmOpcode::SET_FF,
699+
_ => panic!("Invalid bits needed for opcode: {}", bits_needed_opcode),
700+
};
701+
700702
AvmInstruction {
701-
opcode: AvmOpcode::SET,
703+
opcode: set_opcode,
702704
indirect: if indirect { Some(ZEROTH_OPERAND_INDIRECT) } else { Some(ALL_DIRECT) },
703705
tag: Some(tag),
704706
operands: vec![
705-
// const
706-
match tag {
707-
AvmTypeTag::UINT8 => AvmOperand::U8 { value: value as u8 },
708-
AvmTypeTag::UINT16 => AvmOperand::U16 { value: value as u16 },
709-
AvmTypeTag::UINT32 => AvmOperand::U32 { value: value as u32 },
710-
AvmTypeTag::UINT64 => AvmOperand::U64 { value: value as u64 },
711-
AvmTypeTag::UINT128 => AvmOperand::U128 { value },
712-
_ => panic!("Invalid type tag {:?} for set", tag),
713-
},
714-
// dest offset
715-
AvmOperand::U32 { value: dest },
707+
make_operand(bits_needed_opcode, value),
708+
make_operand(bits_needed_mem, &dest),
716709
],
717710
}
718711
}
@@ -1137,8 +1130,6 @@ pub fn map_brillig_pcs_to_avm_pcs(brillig_bytecode: &[BrilligOpcode<FieldElement
11371130
pc_map[0] = 0; // first PC is always 0 as there are no instructions inserted by AVM at start
11381131
for i in 0..brillig_bytecode.len() - 1 {
11391132
let num_avm_instrs_for_this_brillig_instr = match &brillig_bytecode[i] {
1140-
BrilligOpcode::Const { bit_size: BitSize::Field, .. } => 2,
1141-
BrilligOpcode::IndirectConst { bit_size: BitSize::Field, .. } => 2,
11421133
BrilligOpcode::Cast { bit_size: BitSize::Integer(IntegerBitSize::U1), .. } => 3,
11431134
_ => 1,
11441135
};

avm-transpiler/src/utils.rs

+9-7
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use fxhash::FxHashMap as HashMap;
22

33
use acvm::acir::circuit::brillig::BrilligFunctionId;
4-
use acvm::FieldElement;
4+
use acvm::{AcirField, FieldElement};
55
use log::{debug, info, trace};
66

77
use acvm::acir::brillig::Opcode as BrilligOpcode;
@@ -91,13 +91,15 @@ pub fn dbg_print_avm_program(avm_program: &[AvmInstruction]) {
9191
}
9292
}
9393

94-
pub fn make_operand<T: Into<u128> + Copy>(bits: usize, value: &T) -> AvmOperand {
94+
pub fn make_operand<T: Into<FieldElement> + Clone>(bits: usize, value: &T) -> AvmOperand {
95+
let field: FieldElement = value.clone().into();
9596
match bits {
96-
8 => AvmOperand::U8 { value: Into::<u128>::into(*value) as u8 },
97-
16 => AvmOperand::U16 { value: Into::<u128>::into(*value) as u16 },
98-
32 => AvmOperand::U32 { value: Into::<u128>::into(*value) as u32 },
99-
64 => AvmOperand::U64 { value: Into::<u128>::into(*value) as u64 },
100-
128 => AvmOperand::U128 { value: Into::<u128>::into(*value) },
97+
8 => AvmOperand::U8 { value: field.try_to_u32().unwrap() as u8 },
98+
16 => AvmOperand::U16 { value: field.try_to_u32().unwrap() as u16 },
99+
32 => AvmOperand::U32 { value: field.try_to_u32().unwrap() },
100+
64 => AvmOperand::U64 { value: field.try_to_u64().unwrap() },
101+
128 => AvmOperand::U128 { value: field.try_into_u128().unwrap() },
102+
254 => AvmOperand::FF { value: field },
101103
_ => panic!("Invalid operand size for bits: {}", bits),
102104
}
103105
}

0 commit comments

Comments
 (0)