Merge pull request #844 from 0xPolygonMiden/hacka-adv-mem-memory-rang…

…e-from-stack

feat: adv.mem fetchs memory range from stack

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 #### Assembly
 - Added new instruction: `mtree_verify`.
+- [BREAKING] Refactored `adv.mem` decorator to use parameters from operand stack instead of immediate values.
 
 ## 0.5.0 (2023-03-29)
 

assembly/src/assembler/instruction/adv_ops.rs

@@ -21,16 +21,8 @@ pub fn adv_push(span: &mut SpanBuilder, n: u8) -> Result<Option<CodeBlock>, Asse
 // ADVICE INJECTORS
 // ================================================================================================
 
-/// Appends adv.mem.a.n advice injector to the span. This operation copies n number of words from
+/// Appends adv.mem advice injector to the span. This operation copies n number of words from
 /// memory the starting at address a into the advice provider's key-value map.
-///
-/// # Errors
-/// Returns an error is start_addr + num_words > u32::MAX.
-pub fn adv_mem(
-    span: &mut SpanBuilder,
-    start_addr: u32,
-    num_words: u32,
-) -> Result<Option<CodeBlock>, AssemblyError> {
-    validate_param(num_words, 0..=(u32::MAX - start_addr))?;
-    span.add_decorator(Decorator::Advice(AdviceInjector::Memory(start_addr, num_words)))
+pub fn adv_mem(span: &mut SpanBuilder) -> Result<Option<CodeBlock>, AssemblyError> {
+    span.add_decorator(Decorator::Advice(AdviceInjector::Memory))
 }

assembly/src/assembler/instruction/mod.rs

@@ -287,7 +287,7 @@ impl Assembler {
 
             Instruction::AdvU64Div => span.add_decorator(Decorator::Advice(DivResultU64)),
             Instruction::AdvKeyval => span.add_decorator(Decorator::Advice(MapValue)),
-            Instruction::AdvMem(a, n) => adv_ops::adv_mem(span, *a, *n),
+            Instruction::AdvMem => adv_ops::adv_mem(span),
             Instruction::AdvExt2Inv => span.add_decorator(Decorator::Advice(Ext2Inv)),
             Instruction::AdvExt2INTT => span.add_decorator(Decorator::Advice(Ext2INTT)),
 

assembly/src/parsers/adv_ops.rs

@@ -1,5 +1,4 @@
 use super::{
-    parse_checked_param, parse_param,
     Instruction::*,
     Node::{self, Instruction},
     ParsingError, Token,
@@ -13,7 +12,6 @@ use super::{
 /// # Errors
 /// Returns an error if:
 /// - Any of the instructions have a wrong number of parameters.
-/// - adv.mem.a.n has a + n > u32::MAX.
 pub fn parse_adv_inject(op: &Token) -> Result<Node, ParsingError> {
     debug_assert_eq!(op.parts()[0], "adv");
     if op.num_parts() < 2 {
@@ -33,16 +31,12 @@ pub fn parse_adv_inject(op: &Token) -> Result<Node, ParsingError> {
             }
             Ok(Instruction(AdvKeyval))
         }
-        "mem" => match op.num_parts() {
-            0 | 1 => unreachable!(),
-            2 | 3 => Err(ParsingError::missing_param(op)),
-            4 => {
-                let start_addr = parse_param(op, 2)?;
-                let num_words = parse_checked_param(op, 3, 1..=(u32::MAX - start_addr))?;
-                Ok(Instruction(AdvMem(start_addr, num_words)))
+        "mem" => {
+            if op.num_parts() > 2 {
+                return Err(ParsingError::extra_param(op));
             }
-            _ => Err(ParsingError::extra_param(op)),
-        },
+            Ok(Instruction(AdvMem))
+        }
         "ext2inv" => {
             if op.num_parts() > 2 {
                 return Err(ParsingError::extra_param(op));

assembly/src/parsers/nodes.rs

@@ -258,7 +258,7 @@ pub enum Instruction {
 
     AdvU64Div,
     AdvKeyval,
-    AdvMem(u32, u32),
+    AdvMem,
     AdvExt2Inv,
     AdvExt2INTT,
 
@@ -534,7 +534,7 @@ impl fmt::Display for Instruction {
 
             Self::AdvU64Div => write!(f, "adv.u64div"),
             Self::AdvKeyval => write!(f, "adv.keyval"),
-            Self::AdvMem(start_addr, num_words) => write!(f, "adv.mem.{start_addr}.{num_words}"),
+            Self::AdvMem => write!(f, "adv.mem"),
             Self::AdvExt2Inv => write!(f, "adv.ext2inv"),
             Self::AdvExt2INTT => write!(f, "adv.ext2intt"),
 

assembly/src/parsers/serde/deserialization.rs

@@ -326,11 +326,7 @@ impl Deserializable for Instruction {
 
             OpCode::AdvU64Div => Ok(Instruction::AdvU64Div),
             OpCode::AdvKeyval => Ok(Instruction::AdvKeyval),
-            OpCode::AdvMem => {
-                let start_addr = source.read_u32()?;
-                let num_words = source.read_u32()?;
-                Ok(Instruction::AdvMem(start_addr, num_words))
-            }
+            OpCode::AdvMem => Ok(Instruction::AdvMem),
             OpCode::AdvPush => Ok(Instruction::AdvPush(source.read_u8()?)),
             OpCode::AdvLoadW => Ok(Instruction::AdvLoadW),
             OpCode::AdvExt2Inv => Ok(Instruction::AdvExt2Inv),

assembly/src/parsers/serde/serialization.rs

@@ -434,11 +434,7 @@ impl Serializable for Instruction {
 
             Self::AdvU64Div => OpCode::AdvU64Div.write_into(target),
             Self::AdvKeyval => OpCode::AdvKeyval.write_into(target),
-            Self::AdvMem(start_addr, num_words) => {
-                OpCode::AdvMem.write_into(target);
-                target.write_u32(*start_addr);
-                target.write_u32(*num_words);
-            }
+            Self::AdvMem => OpCode::AdvMem.write_into(target),
             Self::AdvPush(v) => {
                 OpCode::AdvPush.write_into(target);
                 target.write_u8(*v);

assembly/src/parsers/tests.rs

@@ -184,11 +184,11 @@ fn test_ast_parsing_adv_ops() {
 
 #[test]
 fn test_ast_parsing_adv_injection() {
-    let source = "begin adv.u64div adv.keyval adv.mem.1.1 end";
+    let source = "begin adv.u64div adv.keyval adv.mem end";
     let nodes: Vec<Node> = vec![
         Node::Instruction(Instruction::AdvU64Div),
         Node::Instruction(Instruction::AdvKeyval),
-        Node::Instruction(Instruction::AdvMem(1, 1)),
+        Node::Instruction(Instruction::AdvMem),
     ];
 
     assert_program_output(source, BTreeMap::new(), nodes);

core/src/operations/decorators/advice.rs

@@ -36,12 +36,11 @@ pub enum AdviceInjector {
     /// stack as key.
     MapValue,
 
-    /// Inserts a list of words that are read from VM memory into the advice map under the key
-    /// specified by the word at the top of the operand stack.
+    /// Reads words from memory range `start_addr .. start_addr + num_words` and insert into the
+    /// advice map under the key `WORD`.
     ///
-    /// The first internal u32 specifies the starting address of the memory region, and the second
-    /// specifies the number of words to be read.
-    Memory(u32, u32),
+    /// Expects the operand stack to be [WORD, start_addr, num_words, ...].
+    Memory,
 
     /// Given an element of quadratic extension field, it computes multiplicative inverse and
     /// push the result into the advice stack.
@@ -60,7 +59,7 @@ impl fmt::Display for AdviceInjector {
             Self::MerkleMerge => write!(f, "merkle_merge"),
             Self::DivResultU64 => write!(f, "div_result_u64"),
             Self::MapValue => write!(f, "map_value"),
-            Self::Memory(start_addr, num_words) => write!(f, "mem({start_addr}, {num_words})"),
+            Self::Memory => write!(f, "mem"),
             Self::Ext2Inv => write!(f, "ext2_inv"),
             Self::Ext2INTT => write!(f, "ext2_intt"),
         }

docs/src/user_docs/assembly/io_operations.md

@@ -38,6 +38,7 @@ In both case the values must still encode valid field elements.
 | adv_push.*n* <br> - *(n cycles)*   | [ ... ]         | [a, ... ]    | $a \leftarrow stack.pop()$ <br> Pops $n$ values from the advice stack and pushes them onto the operand stack. Valid for $n \in \{1, ..., 16\}$. <br> Fails if the advice stack has fewer than $n$ values. |
 | adv_loadw <br> - *(1 cycle)*     | [0, 0, 0, 0, ... ] | [A, ... ] | $A \leftarrow stack.pop(4)$ <br> Pop the next word (4 elements) from the advice stack and overwrites the first word of the operand stack (4 elements) with them. <br> Fails if the advice stack has fewer than $4$ values. |
 | adv_pipe <br> - *(2 cycles)*     | [S2, S1, S0, a, ... ] | [T2, T1, T0, b, ... ] | $[T_0, T_1, T_2] \leftarrow permute(S_0, stack.pop(4), stack.pop(4))$ <br> $b \leftarrow a + 2$ <br> Pops the next two words (8 elements) from the advice stack, inserts them into memory at address $a$ sequentially, overwrites these top 8 elements onto the operand stack, and performs a Rescue Prime Optimized permutation to the top 12 elements of the operand stack. At the end of the operation, the address is incremented by $2$. <br> Fails if the advice stack has fewer than $8$ values. |
+| adv.mem <br> - *(0 cycles)*     | [W, b, a, ... ] | [W, b, a, ... ] | Reads words $data \leftarrow mem[b] .. mem[b + a]$ from memory, and save the data $advice_map[W] \leftarrow data$. |
 
 > **Note**: The opcodes above always push data onto the operand stack so that the first element is placed deepest in the stack. For example, if the data on the stack is `a,b,c,d` and you use the opcode `adv_push.4`, the data will be `d,c,b,a` on your stack. This is also the behavior of the other opcodes.
 

miden/tests/integration/operations/decorators/advice.rs

@@ -133,7 +133,8 @@ fn advice_inject_mem() {
     # copy from memory to advice map
     # the key used is in the reverse order of the field elements in the word at the top of the
     # stack.
-    adv.mem.2.2
+    push.2.4 movdn.4 movdn.4
+    adv.mem
     # State Transition:
     # advice_map: k: [8, 7, 6, 5], v: [4, 3, 2, 1, 8, 7, 6, 5]
 

processor/src/advice/mem_provider.rs

@@ -78,10 +78,8 @@ impl AdviceProvider for MemAdviceProvider {
     }
 
     fn insert_into_map(&mut self, key: Word, values: Vec<Felt>) -> Result<(), ExecutionError> {
-        match self.map.insert(key.into_bytes(), values) {
-            None => Ok(()),
-            Some(_) => Err(ExecutionError::DuplicateAdviceKey(key)),
-        }
+        self.map.insert(key.into_bytes(), values);
+        Ok(())
     }
 
     // ADVISE SETS

processor/src/decorators/mod.rs

@@ -42,9 +42,7 @@ where
             AdviceInjector::MerkleMerge => self.inject_merkle_merge(),
             AdviceInjector::DivResultU64 => self.inject_div_result_u64(),
             AdviceInjector::MapValue => self.inject_map_value(),
-            AdviceInjector::Memory(start_addr, num_words) => {
-                self.inject_mem_values(*start_addr, *num_words)
-            }
+            AdviceInjector::Memory => self.inject_mem_values(),
             AdviceInjector::Ext2Inv => self.inject_ext2_inv_result(),
             AdviceInjector::Ext2INTT => self.inject_ext2_intt_result(),
         }
@@ -165,14 +163,15 @@ where
     ///
     /// # Errors
     /// Returns an error if the key is already present in the advice map.
-    fn inject_mem_values(&mut self, start_addr: u32, num_words: u32) -> Result<(), ExecutionError> {
+    fn inject_mem_values(&mut self) -> Result<(), ExecutionError> {
+        let start_addr = self.stack.get(4).as_int();
+        let end_addr = self.stack.get(5).as_int();
+        let len = end_addr - start_addr;
         let ctx = self.system.ctx();
-        let mut values = Vec::with_capacity(num_words as usize * WORD_SIZE);
-        for i in 0..num_words {
-            let mem_value = self
-                .chiplets
-                .get_mem_value(ctx, (start_addr + i) as u64)
-                .unwrap_or([ZERO; WORD_SIZE]);
+        let mut values = Vec::with_capacity((len as usize) * WORD_SIZE);
+        for i in 0u64..len {
+            let mem_value =
+                self.chiplets.get_mem_value(ctx, start_addr + i).unwrap_or([ZERO; WORD_SIZE]);
             values.extend_from_slice(&mem_value);
         }
         let top_word = self.stack.get_top_word();

processor/src/errors.rs

@@ -25,7 +25,6 @@ pub enum ExecutionError {
     CodeBlockNotFound(Digest),
     CallerNotInSyscall,
     DivideByZero(u32),
-    DuplicateAdviceKey(Word),
     FailedAssertion(u32),
     UninitializedMemoryAddress(u64),
     InvalidFmpValue(Felt, Felt),
@@ -80,10 +79,6 @@ impl Display for ExecutionError {
                 )
             }
             DivideByZero(clk) => write!(fmt, "Division by zero at clock cycle {clk}"),
-            DuplicateAdviceKey(key) => {
-                let hex = to_hex(Felt::elements_as_bytes(key))?;
-                write!(fmt, "Insertion into advice map failed because {hex} already exists")
-            }
             FailedAssertion(clk) => write!(fmt, "Assertion failed at clock cycle {clk}"),
             UninitializedMemoryAddress(address) => {
                 write!(fmt, "Ext2INTT referenced unintialized memory at address {address}")