chore(avm): operands reordering (#10182)

Resolves #10136

avm-transpiler/Cargo.toml

@@ -11,7 +11,6 @@ license = "MIT OR Apache-2.0"
 # local
 acvm = { path = "../noir/noir-repo/acvm-repo/acvm", features = ["bn254"] }
 noirc_errors = { path = "../noir/noir-repo/compiler/noirc_errors" }
-fxhash = "0.2.1"
 
 # external
 base64 = "0.21"

avm-transpiler/README.md

@@ -5,11 +5,50 @@ This component transpiles Aztec public contracts code from Noir's Brillig byteco
 ## Build
 
 ```
-./boostrap.sh
+./bootstrap.sh
 ```
 
 ## Run
 
 ```
 cargo run <aztec-contract-artifact-json> <transpiled-output-json>
 ```
+
+## Testing Transpiler Changes
+
+After bootstrap in `avm-transpiler`, go to `noir-contracts` and only compile avm_test_contract with:
+
+```
+nargo compile --package avm_test_contract --inliner-aggressiveness=0 --silence-warnings
+```
+
+Important: use the right nargo binary located in
+`aztec-packages/noir/noir-repo/target/release/nargo`
+If required, build nargo by going in `noir/noir-repo` and run
+`cargo build --release`.
+
+Then, transpile it:
+
+```
+scripts/transpile.sh
+```
+
+Go to yarn-project/simulator and run:
+
+```
+yarn build:fast
+```
+
+This takes in the TS generated by the compilation and transpilation.
+
+Finally, run
+
+```
+yarn test src/avm/avm_simulator.test.ts
+```
+
+To test against some .cpp changes, compile the bb binary and run bb prover test:
+
+```
+yarn test src/avm_proving.test.ts
+```

avm-transpiler/src/instructions.rs

@@ -22,8 +22,12 @@ pub struct AvmInstruction {
     /// Its usage will depend on the instruction.
     pub tag: Option<AvmTypeTag>,
 
-    /// Different instructions have different numbers of operands
+    /// Different instructions have different numbers of operands. These operands contain
+    /// memory addresses.
     pub operands: Vec<AvmOperand>,
+
+    // Operands which are immediate, i.e., contain hardcoded constants.
+    pub immediates: Vec<AvmOperand>,
 }
 
 impl Display for AvmInstruction {
@@ -32,35 +36,46 @@ impl Display for AvmInstruction {
         if let Some(indirect) = &self.indirect {
             write!(f, ", indirect: {}", indirect)?;
         }
-        // This will be either inTag or dstTag depending on the operation
-        if let Some(dst_tag) = self.tag {
-            write!(f, ", tag: {}", dst_tag as u8)?;
-        }
         if !self.operands.is_empty() {
             write!(f, ", operands: [")?;
             for operand in &self.operands {
                 write!(f, "{operand}, ")?;
             }
             write!(f, "]")?;
         };
+        // This will be either inTag or dstTag depending on the operation
+        if let Some(dst_tag) = self.tag {
+            write!(f, ", tag: {}", dst_tag as u8)?;
+        }
+        if !self.immediates.is_empty() {
+            write!(f, ", immediates: [")?;
+            for immediate in &self.immediates {
+                write!(f, "{immediate}, ")?;
+            }
+            write!(f, "]")?;
+        };
         Ok(())
     }
 }
 
 impl AvmInstruction {
     /// Bytes representation for generating AVM bytecode
+    /// Order: INDIRECT, OPERANDS, TAG, IMMEDIATES
     pub fn to_bytes(&self) -> Vec<u8> {
         let mut bytes = Vec::new();
         bytes.push(self.opcode as u8);
         if let Some(indirect) = &self.indirect {
             bytes.extend_from_slice(&indirect.to_be_bytes());
         }
+        for operand in &self.operands {
+            bytes.extend_from_slice(&operand.to_be_bytes());
+        }
         // This will be either inTag or dstTag depending on the operation
         if let Some(tag) = self.tag {
             bytes.extend_from_slice(&(tag as u8).to_be_bytes());
         }
-        for operand in &self.operands {
-            bytes.extend_from_slice(&operand.to_be_bytes());
+        for immediate in &self.immediates {
+            bytes.extend_from_slice(&immediate.to_be_bytes());
         }
         bytes
     }
@@ -84,6 +99,7 @@ impl Default for AvmInstruction {
             indirect: None,
             tag: None,
             operands: vec![],
+            immediates: vec![],
         }
     }
 }
@@ -102,9 +118,8 @@ pub enum AvmTypeTag {
     INVALID,
 }
 
-/// Operands are usually 32 bits (offsets or jump destinations)
-/// Constants (as used by the SET instruction) can have size
-/// different from 32 bits
+/// Operands are usually 8, 16 and 32 bits (offsets)
+/// Immediates (as used by the SET instruction) can have different sizes
 #[allow(non_camel_case_types)]
 pub enum AvmOperand {
     U8 { value: u8 },

avm-transpiler/src/transpile.rs

@@ -1,5 +1,4 @@
 use acvm::acir::brillig::{BitSize, IntegerBitSize, Opcode as BrilligOpcode};
-use fxhash::FxHashMap as HashMap;
 use std::collections::BTreeMap;
 
 use acvm::acir::circuit::BrilligOpcodeLocation;
@@ -90,12 +89,12 @@ pub fn brillig_to_avm(brillig_bytecode: &[BrilligOpcode<FieldElement>]) -> (Vec<
                             .direct_operand(destination)
                             .build(),
                     ),
-                    tag: None,
                     operands: vec![
                         make_operand(bits_needed, &lhs.to_usize()),
                         make_operand(bits_needed, &rhs.to_usize()),
                         make_operand(bits_needed, &destination.to_usize()),
                     ],
+                    ..Default::default()
                 });
             }
             BrilligOpcode::BinaryIntOp { destination, op, lhs, rhs, .. } => {
@@ -178,12 +177,12 @@ pub fn brillig_to_avm(brillig_bytecode: &[BrilligOpcode<FieldElement>]) -> (Vec<
                             .direct_operand(destination)
                             .build(),
                     ),
-                    tag: None,
                     operands: vec![
                         make_operand(bits_needed, &lhs.to_usize()),
                         make_operand(bits_needed, &rhs.to_usize()),
                         make_operand(bits_needed, &destination.to_usize()),
                     ],
+                    ..Default::default()
                 });
             }
             BrilligOpcode::Not { destination, source, .. } => {
@@ -207,7 +206,7 @@ pub fn brillig_to_avm(brillig_bytecode: &[BrilligOpcode<FieldElement>]) -> (Vec<
                         make_operand(bits_needed, &source.to_usize()),
                         make_operand(bits_needed, &destination.to_usize()),
                     ],
-                    tag: None,
+                    ..Default::default()
                 });
             }
             BrilligOpcode::CalldataCopy { destination_address, size_address, offset_address } => {
@@ -236,7 +235,7 @@ pub fn brillig_to_avm(brillig_bytecode: &[BrilligOpcode<FieldElement>]) -> (Vec<
                 assert!(location.num_bits() <= 32);
                 avm_instrs.push(AvmInstruction {
                     opcode: AvmOpcode::JUMP_32,
-                    operands: vec![AvmOperand::BRILLIG_LOCATION { brillig_pc: *location as u32 }],
+                    immediates: vec![AvmOperand::BRILLIG_LOCATION { brillig_pc: *location as u32 }],
                     ..Default::default()
                 });
             }
@@ -247,10 +246,8 @@ pub fn brillig_to_avm(brillig_bytecode: &[BrilligOpcode<FieldElement>]) -> (Vec<
                     indirect: Some(
                         AddressingModeBuilder::default().direct_operand(condition).build(),
                     ),
-                    operands: vec![
-                        AvmOperand::BRILLIG_LOCATION { brillig_pc: *location as u32 },
-                        make_operand(16, &condition.to_usize()),
-                    ],
+                    operands: vec![make_operand(16, &condition.to_usize())],
+                    immediates: vec![AvmOperand::BRILLIG_LOCATION { brillig_pc: *location as u32 }],
                     ..Default::default()
                 });
             }
@@ -300,7 +297,7 @@ pub fn brillig_to_avm(brillig_bytecode: &[BrilligOpcode<FieldElement>]) -> (Vec<
                 assert!(location.num_bits() <= 32);
                 avm_instrs.push(AvmInstruction {
                     opcode: AvmOpcode::INTERNALCALL,
-                    operands: vec![AvmOperand::BRILLIG_LOCATION { brillig_pc: *location as u32 }],
+                    immediates: vec![AvmOperand::BRILLIG_LOCATION { brillig_pc: *location as u32 }],
                     ..Default::default()
                 });
             }
@@ -353,8 +350,8 @@ pub fn brillig_to_avm(brillig_bytecode: &[BrilligOpcode<FieldElement>]) -> (Vec<
         .into_iter()
         .map(|i| match i.opcode {
             AvmOpcode::JUMP_32 | AvmOpcode::JUMPI_32 | AvmOpcode::INTERNALCALL => {
-                let new_operands = i
-                    .operands
+                let new_immediates = i
+                    .immediates
                     .into_iter()
                     .map(|o| match o {
                         AvmOperand::BRILLIG_LOCATION { brillig_pc } => {
@@ -365,7 +362,7 @@ pub fn brillig_to_avm(brillig_bytecode: &[BrilligOpcode<FieldElement>]) -> (Vec<
                         _ => o,
                     })
                     .collect::<Vec<AvmOperand>>();
-                AvmInstruction { operands: new_operands, ..i }
+                AvmInstruction { immediates: new_immediates, ..i }
             }
             _ => i,
         })
@@ -832,10 +829,8 @@ fn handle_getter_instruction(
     avm_instrs.push(AvmInstruction {
         opcode: AvmOpcode::GETENVVAR_16,
         indirect: Some(AddressingModeBuilder::default().direct_operand(&dest_offset).build()),
-        operands: vec![
-            AvmOperand::U8 { value: var_idx as u8 },
-            AvmOperand::U16 { value: dest_offset.to_usize() as u16 },
-        ],
+        operands: vec![AvmOperand::U16 { value: dest_offset.to_usize() as u16 }],
+        immediates: vec![AvmOperand::U8 { value: var_idx as u8 }],
         ..Default::default()
     });
 }
@@ -882,10 +877,8 @@ fn generate_set_instruction(
             Some(AddressingModeBuilder::default().direct_operand(dest).build())
         },
         tag: Some(tag),
-        operands: vec![
-            make_operand(bits_needed_opcode, value),
-            make_operand(bits_needed_mem, &(dest.to_usize())),
-        ],
+        operands: vec![make_operand(bits_needed_mem, &(dest.to_usize()))],
+        immediates: vec![make_operand(bits_needed_opcode, value)],
     }
 }
 
@@ -924,6 +917,7 @@ fn generate_cast_instruction(
             make_operand(bits_needed, &(source.to_usize())),
             make_operand(bits_needed, &(destination.to_usize())),
         ],
+        ..Default::default()
     }
 }
 
@@ -1087,14 +1081,16 @@ fn handle_black_box_function(avm_instrs: &mut Vec<AvmInstruction>, operation: &B
                         .direct_operand(radix)
                         .build(),
                 ),
-                tag: None,
                 operands: vec![
                     AvmOperand::U16 { value: input_offset as u16 },
                     AvmOperand::U16 { value: output_offset as u16 },
                     AvmOperand::U16 { value: radix_offset as u16 },
+                ],
+                immediates: vec![
                     AvmOperand::U16 { value: num_limbs as u16 },
                     AvmOperand::U8 { value: *output_bits as u8 },
                 ],
+                ..Default::default()
             });
         }
         // This will be changed to utilise relative memory offsets
@@ -1202,10 +1198,10 @@ fn handle_debug_log(
         ),
         operands: vec![
             AvmOperand::U16 { value: message_offset.to_usize() as u16 },
-            AvmOperand::U16 { value: message_size as u16 },
             AvmOperand::U16 { value: fields_offset_ptr.to_usize() as u16 },
             AvmOperand::U16 { value: fields_size_ptr.to_usize() as u16 },
         ],
+        immediates: vec![AvmOperand::U16 { value: message_size as u16 }],
         ..Default::default()
     });
 }
@@ -1462,11 +1458,11 @@ fn handle_get_contract_instance(
                 .build(),
         ),
         operands: vec![
-            AvmOperand::U8 { value: member_idx as u8 },
             AvmOperand::U16 { value: address_offset.to_usize() as u16 },
             AvmOperand::U16 { value: dest_offset.to_usize() as u16 },
             AvmOperand::U16 { value: exists_offset.to_usize() as u16 },
         ],
+        immediates: vec![AvmOperand::U8 { value: member_idx as u8 }],
         ..Default::default()
     });
 }