chore(acir)!: Remove Block, RAM and ROM opcodes (#457)

acir/src/circuit/opcodes.rs

@@ -6,10 +6,10 @@ use crate::native_types::{Expression, Witness};
 use serde::{Deserialize, Serialize};
 
 mod black_box_function_call;
-mod block;
+mod memory_operation;
 
 pub use black_box_function_call::{BlackBoxFuncCall, FunctionInput};
-pub use block::{BlockId, MemOp, MemoryBlock};
+pub use memory_operation::{BlockId, MemOp};
 
 #[derive(Clone, PartialEq, Eq, Serialize, Deserialize)]
 pub enum Opcode {
@@ -19,24 +19,8 @@ pub enum Opcode {
     /// Often used for exposing more efficient implementations of SNARK-unfriendly computations.  
     BlackBoxFuncCall(BlackBoxFuncCall),
     Directive(Directive),
-    /// Abstract read/write operations on a block of data. In particular;
-    /// - It does not require an initialization phase
-    /// - Operations do not need to be constant, they can be any expression which resolves to 0 or 1.
-    Block(MemoryBlock),
-    /// Same as Block, but it starts with an initialization phase and then have only read operation
-    /// - init: write operations with index from 0..MemoryBlock.len
-    /// - after MemoryBlock.len; all operations are read
-    ///
-    /// ROM can be more efficiently handled because we do not need to check for the operation value (which is always 0).
-    ROM(MemoryBlock),
-    /// Same as ROM, but can have read or write operations
-    /// - init = write operations with index 0..MemoryBlock.len
-    /// - after MemoryBlock.len, all operations are constant expressions (0 or 1)
-    // TODO(#319): Review this comment and generalize it to be useful for other backends.
-    // RAM is required for acvm-backend-barretenberg as dynamic memory implementation in Barretenberg requires an initialization phase and can only handle constant values for operations.
-    RAM(MemoryBlock),
     Brillig(Brillig),
-    /// Atomic operation on a memory block
+    /// Atomic operation on a block of memory
     MemoryOp {
         block_id: BlockId,
         op: MemOp,
@@ -70,9 +54,6 @@ impl Opcode {
             Opcode::Arithmetic(_) => "arithmetic",
             Opcode::Directive(directive) => directive.name(),
             Opcode::BlackBoxFuncCall(g) => g.name(),
-            Opcode::Block(_) => "block",
-            Opcode::RAM(_) => "ram",
-            Opcode::ROM(_) => "rom",
             Opcode::Brillig(_) => "brillig",
             Opcode::MemoryOp { .. } => "mem",
             Opcode::MemoryInit { .. } => "init memory block",
@@ -171,18 +152,6 @@ impl std::fmt::Display for Opcode {
                     witnesses.last().unwrap().witness_index()
                 ),
             },
-            Opcode::Block(block) => {
-                write!(f, "BLOCK ")?;
-                write!(f, "(id: {}, len: {}) ", block.id.0, block.trace.len())
-            }
-            Opcode::ROM(block) => {
-                write!(f, "ROM ")?;
-                write!(f, "(id: {}, len: {}) ", block.id.0, block.trace.len())
-            }
-            Opcode::RAM(block) => {
-                write!(f, "RAM ")?;
-                write!(f, "(id: {}, len: {}) ", block.id.0, block.trace.len())
-            }
             Opcode::Brillig(brillig) => {
                 write!(f, "BRILLIG: ")?;
                 writeln!(f, "inputs: {:?}", brillig.inputs)?;

acir/src/circuit/opcodes/memory_operation.rs

@@ -0,0 +1,28 @@
+use crate::native_types::{Expression, Witness};
+use serde::{Deserialize, Serialize};
+
+#[derive(Clone, PartialEq, Eq, Serialize, Deserialize, Hash, Copy, Default)]
+pub struct BlockId(pub u32);
+
+/// Operation on a block of memory
+/// We can either write or read at an index in memory
+#[derive(Clone, PartialEq, Eq, Serialize, Deserialize, Debug)]
+pub struct MemOp {
+    /// Can be 0 (read) or 1 (write)
+    pub operation: Expression,
+    pub index: Expression,
+    pub value: Expression,
+}
+
+impl MemOp {
+    /// Creates a `MemOp` which reads from memory at `index` and inserts the read value
+    /// into the [`WitnessMap`][crate::native_types::WitnessMap] at `witness`
+    pub fn read_at_mem_index(index: Expression, witness: Witness) -> Self {
+        MemOp { operation: Expression::zero(), index, value: witness.into() }
+    }
+
+    /// Creates a `MemOp` which writes the [`Expression`] `value` into memory at `index`.
+    pub fn write_to_mem_index(index: Expression, value: Expression) -> Self {
+        MemOp { operation: Expression::one(), index, value }
+    }
+}

acvm/src/compiler/mod.rs

@@ -199,10 +199,6 @@ pub fn compile(
                 new_opcode_labels.push(opcode_label[index]);
                 transformed_gates.push(opcode.clone());
             }
-
-            Opcode::Block(_) | Opcode::ROM(_) | Opcode::RAM(_) => {
-                unimplemented!("Stepwise execution is not compatible with {}", opcode.name())
-            }
             Opcode::Brillig(brillig) => {
                 for output in &brillig.outputs {
                     match output {

acvm/src/compiler/transformers/fallback.rs

@@ -21,12 +21,7 @@ impl FallbackTransformer {
 
         for (idx, opcode) in acir.opcodes.into_iter().enumerate() {
             match &opcode {
-                Opcode::Arithmetic(_)
-                | Opcode::Directive(_)
-                | Opcode::Brillig(_)
-                | Opcode::Block(_)
-                | Opcode::ROM(_)
-                | Opcode::RAM(_) => {
+                Opcode::Arithmetic(_) | Opcode::Directive(_) | Opcode::Brillig(_) => {
                     // directive, arithmetic expression or blocks are handled by acvm
                     new_opcode_labels.push(opcode_labels[idx]);
                     acir_supported_opcodes.push(opcode);

acvm/src/pwg/block.rs → acvm/src/pwg/memory_op.rs

@@ -11,15 +11,13 @@ use super::{arithmetic::ArithmeticSolver, get_value, insert_value, witness_to_va
 
 type MemoryIndex = u32;
 
-/// Maintains the state for solving Block opcode
-/// block_value is the value of the Block at the solved_operations step
-/// solved_operations is the number of solved elements in the block
+/// Maintains the state for solving [`MemoryInit`][`acir::circuit::Opcode::MemoryInit`] and [`MemoryOp`][`acir::circuit::Opcode::MemoryOp`] opcodes.
 #[derive(Default)]
-pub(super) struct BlockSolver {
+pub(super) struct MemoryOpSolver {
     block_value: HashMap<MemoryIndex, FieldElement>,
 }
 
-impl BlockSolver {
+impl MemoryOpSolver {
     fn write_memory_index(&mut self, index: MemoryIndex, value: FieldElement) {
         self.block_value.insert(index, value);
     }
@@ -100,7 +98,7 @@ mod tests {
         FieldElement,
     };
 
-    use super::BlockSolver;
+    use super::MemoryOpSolver;
 
     #[test]
     fn test_solver() {
@@ -117,7 +115,7 @@ mod tests {
             MemOp::read_at_mem_index(FieldElement::one().into(), Witness(4)),
         ];
 
-        let mut block_solver = BlockSolver::default();
+        let mut block_solver = MemoryOpSolver::default();
         block_solver.init(&init, &initial_witness).unwrap();
 
         for op in trace {

acvm/src/pwg/mod.rs

@@ -11,8 +11,8 @@ use acir::{
 use blackbox_solver::BlackBoxResolutionError;
 
 use self::{
-    arithmetic::ArithmeticSolver, block::BlockSolver, brillig::BrilligSolver,
-    directives::solve_directives,
+    arithmetic::ArithmeticSolver, brillig::BrilligSolver, directives::solve_directives,
+    memory_op::MemoryOpSolver,
 };
 use crate::{BlackBoxFunctionSolver, Language};
 
@@ -26,7 +26,7 @@ mod brillig;
 mod directives;
 // black box functions
 mod blackbox;
-mod block;
+mod memory_op;
 
 pub use brillig::ForeignCallWaitInfo;
 
@@ -98,8 +98,8 @@ pub struct ACVM<B: BlackBoxFunctionSolver> {
 
     backend: B,
 
-    /// Stores the solver for each [block][`Opcode::Block`] opcode. This persists their internal state to prevent recomputation.
-    block_solvers: HashMap<BlockId, BlockSolver>,
+    /// Stores the solver for memory operations acting on blocks of memory disambiguated by [block][`BlockId`].
+    block_solvers: HashMap<BlockId, MemoryOpSolver>,
 
     /// A list of opcodes which are to be executed by the ACVM.
     opcodes: Vec<Opcode>,
@@ -227,9 +227,6 @@ impl<B: BlackBoxFunctionSolver> ACVM<B> {
                     res => res.map(|_| ()),
                 }
             }
-            Opcode::Block(_) | Opcode::ROM(_) | Opcode::RAM(_) => {
-                panic!("Block, ROM and RAM opcodes are not supported by stepwise ACVM")
-            }
         };
         match resolution {
             Ok(()) => {
@@ -350,8 +347,8 @@ pub fn default_is_opcode_supported(language: Language) -> fn(&Opcode) -> bool {
     // The ones which are not supported, the acvm compiler will
     // attempt to transform into supported gates. If these are also not available
     // then a compiler error will be emitted.
-    fn plonk_is_supported(opcode: &Opcode) -> bool {
-        !matches!(opcode, Opcode::Block(_))
+    fn plonk_is_supported(_opcode: &Opcode) -> bool {
+        true
     }
 
     match language {