change fibonacci test to memcpy test

Author: pjabbarzade

Cargo.lock

@@ -1,3 +1,4 @@
 [app_vm_config.rv32i]
 [app_vm_config.rv32m]
 [app_vm_config.io]
+[app_vm_config.memcpy]

benchmarks/guest/fibonacci/openvm.toml

@@ -1,14 +1,44 @@
-use openvm::io::{read, reveal_u32};
+use core::ptr;
 
-pub fn main() {
-    let n: u64 = read();
-    let mut a: u64 = 0;
-    let mut b: u64 = 1;
-    for _ in 0..n {
-        let c: u64 = a.wrapping_add(b);
-        a = b;
-        b = c;
+openvm::entry!(main);
+
+/// Moves all the elements of `src` into `dst`, leaving `src` empty.
+#[no_mangle]
+pub fn append<T>(dst: &mut [T], src: &mut [T], shift: usize) {
+    let src_len = src.len();
+    let dst_len = dst.len();
+
+    unsafe {
+        // The call to add is always safe because `Vec` will never
+        // allocate more than `isize::MAX` bytes.
+        let dst_ptr = dst.as_mut_ptr().wrapping_add(shift);
+        let src_ptr = src.as_ptr();
+        println!("dst_ptr: {:?}", dst_ptr);
+        println!("src_ptr: {:?}", src_ptr);
+        println!("src_len: {:?}", src_len);
+
+        // The two regions cannot overlap because mutable references do
+        // not alias, and two different vectors cannot own the same
+        // memory.
+        ptr::copy_nonoverlapping(src_ptr, dst_ptr, src_len);
     }
-    reveal_u32(a as u32, 0);
-    reveal_u32((a >> 32) as u32, 1);
 }
+
+pub fn main() {
+    let mut a: [u8; 1000] = [1; 1000];
+    let mut b: [u8; 500] = [2; 500];
+
+    let shift: usize = 0;
+    append(&mut a, &mut b, shift);
+
+    for i in 0..1000 {
+        if i < shift || i >= shift + b.len() {
+            assert_eq!(a[i], 1);
+        } else {
+            assert_eq!(a[i], 2);
+        }
+    }
+
+    println!("a: {:?}", a);
+    println!("b: {:?}", b);
+}

benchmarks/guest/fibonacci/src/main.rs

@@ -255,7 +255,6 @@ memcpy:
 	sb	a6, 2(a3)
 	addi	a2, a2, -3
 	addi	a3, a4, 16
-	li	a4, 16
 .LBBmemcpy0_9:
 	memcpy_loop 1
 	addi	a4, a3, -13
@@ -268,7 +267,6 @@ memcpy:
 .LBBmemcpy0_12:
 	li	a1, 16
 	bltu	a2, a1, .LBBmemcpy0_15
-	li	a1, 15
 .LBBmemcpy0_14:
 	memcpy_loop 0
 .LBBmemcpy0_15:
@@ -294,7 +292,6 @@ memcpy:
 	sb	a5, 0(a3)
 	addi	a2, a2, -1
 	addi	a3, a4, 16
-	li	a4, 18
 .LBBmemcpy0_20:
 	memcpy_loop 3
 	addi	a4, a3, -15
@@ -307,7 +304,6 @@ memcpy:
 	sb	a6, 1(a3)
 	addi	a2, a2, -2
 	addi	a3, a4, 16
-	li	a4, 17
 .LBBmemcpy0_23:
 	memcpy_loop 2
 	addi	a4, a3, -14

crates/toolchain/openvm/src/memcpy.s

@@ -11,28 +11,20 @@ memcpy_loop shift
 
 Where `shift` is an immediate value (0, 1, 2, or 3) representing the byte alignment shift.
 
-### RISC-V Encoding
-- **Opcode**: `0x73` (custom opcode)
-- **Funct3**: `0x0` (custom funct3)
-- **Immediate**: 12-bit signed immediate for shift value
-- **Format**: I-type instruction
 
 ### Usage
 The `memcpy_loop` instruction is designed to replace repetitive shift-handling code in memcpy implementations. Instead of having separate code blocks for each shift value, you can use a single instruction:
 
 ```assembly
-# Instead of this repetitive code:
-.Lshift_1:
-    lw      a5, 0(a4)
-    sb      a5, 0(a3)
-    srli    a1, a5, 8
-    sb      a1, 1(a3)
-    # ... more shift handling code
-
-# You can use:
 memcpy_loop 1    # Handles shift=1 case
 ```
 
+Note that you must define `memcpy_loop` before using it. For example, in [memcpy.s](../../crates/toolchain/openvm/src/memcpy.s) it is defined at the beginning of the assembly code as follows:
+```assembly
+.macro memcpy_loop shift
+		.word 0x00000072 | (\shift << 12)  # opcode 0x72 + shift in immediate field (bits 12-31)
+```
+
 ### Benefits
 1. **Code Size Reduction**: Eliminates repetitive shift-handling code
 2. **Performance**: Optimized implementation in the circuit layer
@@ -84,3 +76,8 @@ RISC-V Assembly → Transpiler Extension → OpenVM Instruction → MemcpyIterat
 The extension provides:
 - **Transpiler**: `extensions/memcpy/transpiler/` - Translates RISC-V to OpenVM
 - **Circuit**: `extensions/memcpy/circuit/` - Implements the instruction logic
+
+
+# References
+
+- Official Keccak [spec summary](https://keccak.team/keccak_specs_summary.html)

extensions/memcpy/README.md

@@ -18,7 +18,7 @@ use openvm_circuit::{
             MemoryWriteAuxCols, MemoryWriteBytesAuxRecord,
         },
         online::{GuestMemory, TracingMemory},
-        MemoryAddress, MemoryAuxColsFactory,
+        MemoryAddress, MemoryAuxColsFactory, POINTER_MAX_BITS,
     },
 };
 use openvm_circuit_primitives::{
@@ -441,6 +441,7 @@ where
         );
         let mut len = read_rv32_register(state.memory.data(), A2_REGISTER_PTR as u32);
 
+        // Create a record with var_size = ((len - shift) >> 4) + 1 which is the number of rows in iteration trace
         let record = state.ctx.alloc(MultiRowLayout::new(MemcpyIterMetadata {
             num_rows: ((len - shift as u32) >> 4) as usize + 1,
         }));
@@ -449,7 +450,11 @@ where
         record.inner.shift = shift;
         record.inner.from_pc = *state.pc;
         record.inner.from_timestamp = state.memory.timestamp;
+        record.inner.dest = dest;
+        record.inner.source = source;
+        record.inner.len = len;
 
+        // Fill record.var for the first row of iteration trace
         if shift != 0 {
             source -= 12;
             record.var[0].data[3] = tracing_read(
@@ -460,6 +465,7 @@ where
             );
         };
 
+        // Fill record.var for the rest of the rows of iteration trace
         let mut idx = 1;
         while len - shift as u32 > 15 {
             let writes_data: [[u8; MEMCPY_LOOP_NUM_LIMBS]; 4] = array::from_fn(|i| {
@@ -540,9 +546,9 @@ where
             &mut len_data,
         );
 
-        record.inner.dest = u32::from_le_bytes(dest_data);
-        record.inner.source = u32::from_le_bytes(source_data);
-        record.inner.len = u32::from_le_bytes(len_data);
+        debug_assert_eq!(record.inner.dest, u32::from_le_bytes(dest_data));
+        debug_assert_eq!(record.inner.source, u32::from_le_bytes(source_data));
+        debug_assert_eq!(record.inner.len, u32::from_le_bytes(len_data));
 
         *state.pc = state.pc.wrapping_add(DEFAULT_PC_STEP);
 
@@ -580,7 +586,7 @@ impl<F: PrimeField32> TraceFiller<F> for MemcpyIterFiller {
             num_loops += 1;
             num_iters += num_rows;
         }
-        // tracing::info!("num_loops: {:?}, num_iters: {:?}", num_loops, num_iters);
+        tracing::info!("num_loops: {:?}, num_iters: {:?}, sizes: {:?}", num_loops, num_iters, sizes);
 
         chunks
             .par_iter_mut()
@@ -594,6 +600,7 @@ impl<F: PrimeField32> TraceFiller<F> for MemcpyIterFiller {
                     )
                 };
 
+                tracing::info!("shift: {:?}", record.inner.shift);
                 // Fill memcpy loop record
                 self.memcpy_loop_chip.add_new_loop(
                     mem_helper,
@@ -606,6 +613,7 @@ impl<F: PrimeField32> TraceFiller<F> for MemcpyIterFiller {
                     record.inner.register_aux.clone(),
                 );
 
+                // Calculate the timestamp for the last memory access
                 // 4 reads + 4 writes per iteration + (shift != 0) read for the loop header
                 let timestamp = record.inner.from_timestamp
                     + ((num_rows - 1) << 3) as u32
@@ -906,6 +914,7 @@ unsafe fn execute_e12_impl<F: PrimeField32, CTX: ExecutionCtxTrait>(
 ) -> u32 {
     let shift = pre_compute.c;
     let mut height = 1;
+    // Read dest and source from registers
     let (dest, source) = if shift == 0 {
         (
             vm_state.vm_read::<u8, 4>(RV32_REGISTER_AS, A3_REGISTER_PTR as u32),
@@ -917,19 +926,31 @@ unsafe fn execute_e12_impl<F: PrimeField32, CTX: ExecutionCtxTrait>(
             vm_state.vm_read::<u8, 4>(RV32_REGISTER_AS, A3_REGISTER_PTR as u32),
         )
     };
+    // Read length from a2 register
     let len = vm_state.vm_read::<u8, 4>(RV32_REGISTER_AS, A2_REGISTER_PTR as u32);
 
     let mut dest = u32::from_le_bytes(dest);
-    let mut source = u32::from_le_bytes(source);
+    let mut source = u32::from_le_bytes(source) - 12 * (shift != 0) as u32;
     let mut len = u32::from_le_bytes(len);
 
+    // Check address ranges are valid
+    debug_assert!(dest < (1 << POINTER_MAX_BITS));
+    debug_assert!((source - 4 * (shift != 0) as u32) < (1 << POINTER_MAX_BITS));
+    let to_dest = dest + ((len - shift as u32) & !15);
+    let to_source = source + ((len - shift as u32) & !15);
+    debug_assert!(to_dest <= (1 << POINTER_MAX_BITS));
+    debug_assert!(to_source <= (1 << POINTER_MAX_BITS));
+    // Make sure the destination and source are not overlapping
+    debug_assert!(to_dest <= source || to_source <= dest);
+
+    // Read the previous data from memory if shift != 0
     let mut prev_data = if shift == 0 {
         [0; 4]
     } else {
-        source -= 12;
         vm_state.vm_read::<u8, 4>(RV32_MEMORY_AS, source - 4)
     };
 
+    // Run iterations
     while len - shift as u32 > 15 {
         for i in 0..4 {
             let data = vm_state.vm_read::<u8, 4>(RV32_MEMORY_AS, source + 4 * i);

extensions/memcpy/circuit/src/iteration.rs

@@ -181,6 +181,7 @@ impl<AB: InteractionBuilder> Air<AB> for MemcpyLoopAir {
             .eval(builder, local.is_valid);
 
         // Generate 16-bit limbs for range checking
+        // dest, to_dest, source - 12 * is_shift_non_zero, to_source - 12 * is_shift_non_zero
         let dest_u16_limbs = u8_word_to_u16(local.dest);
         let to_dest_u16_limbs = u8_word_to_u16(local.to_dest);
         let source_u16_limbs = [
@@ -213,12 +214,14 @@ impl<AB: InteractionBuilder> Air<AB> for MemcpyLoopAir {
         ];
 
         range_check_data.iter().for_each(|data| {
+            // Check the low 16 bits of dest and source, make sure they are multiple of 4
             self.range_bus
                 .range_check(
                     data[0].clone() * AB::F::from_canonical_u32(4).inverse(),
                     MEMCPY_LOOP_LIMB_BITS * 2 - 2,
                 )
                 .eval(builder, local.is_valid);
+            // Check the high 16 bits of dest and source, make sure they are in the range [0, 2^pointer_max_bits - 2^MEMCPY_LOOP_LIMB_BITS)
             self.range_bus
                 .range_check(
                     data[1].clone(),
@@ -395,6 +398,7 @@ impl MemcpyLoopChip {
         let height = next_power_of_two_or_zero(self.records.lock().unwrap().len());
         let mut rows = F::zero_vec(height * NUM_MEMCPY_LOOP_COLS);
 
+        // TODO: run in parallel
         for (i, record) in self.records.lock().unwrap().iter().enumerate() {
             let row = &mut rows[i * NUM_MEMCPY_LOOP_COLS..(i + 1) * NUM_MEMCPY_LOOP_COLS];
             let cols: &mut MemcpyLoopCols<F> = row.borrow_mut();