Memory Align

core/src/zisk_required_operation.rs

@@ -1,4 +1,4 @@
-use std::collections::HashMap;
+use std::{collections::HashMap, fmt};
 
 #[derive(Clone)]
 pub struct ZiskRequiredOperation {
@@ -8,7 +8,7 @@ pub struct ZiskRequiredOperation {
     pub b: u64,
 }
 
-#[derive(Clone, Debug)]
+#[derive(Clone)]
 pub struct ZiskRequiredMemory {
     pub step: u64,
     pub is_write: bool,
@@ -17,6 +17,22 @@ pub struct ZiskRequiredMemory {
     pub value: u64,
 }
 
+impl fmt::Debug for ZiskRequiredMemory {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        let label = if self.is_write { "WR" } else { "RD" };
+        write!(
+            f,
+            "{0} addr:{1:#08X}({1}) offset:{5} with:{2} value:{3:#016X}({3}) step:{4}",
+            label,
+            self.address,
+            self.width,
+            self.value,
+            self.step,
+            self.address & 0x07
+        )
+    }
+}
+
 #[derive(Clone, Default)]
 pub struct ZiskRequired {
     pub arith: Vec<ZiskRequiredOperation>,

emulator/src/emu.rs

@@ -98,7 +98,6 @@ impl<'a> Emu<'a> {
         &mut self,
         instruction: &ZiskInst,
         emu_mem: &mut Vec<ZiskRequiredMemory>,
-        is_aligned: bool,
     ) {
         match instruction.a_src {
             SRC_C => self.ctx.inst_ctx.a = self.ctx.inst_ctx.c,
@@ -109,16 +108,14 @@ impl<'a> Emu<'a> {
                 }
                 self.ctx.inst_ctx.a = self.ctx.inst_ctx.mem.read(addr, 8);
 
-                if is_aligned == Self::is_8_aligned(addr, 8) {
-                    let required_memory = ZiskRequiredMemory {
-                        step: self.ctx.inst_ctx.step,
-                        is_write: false,
-                        address: addr,
-                        width: 8,
-                        value: self.ctx.inst_ctx.a,
-                    };
-                    emu_mem.push(required_memory);
-                }
+                let required_memory = ZiskRequiredMemory {
+                    step: self.ctx.inst_ctx.step,
+                    is_write: false,
+                    address: addr,
+                    width: 8,
+                    value: self.ctx.inst_ctx.a,
+                };
+                emu_mem.push(required_memory);
             }
             SRC_IMM => {
                 self.ctx.inst_ctx.a = instruction.a_offset_imm0 | (instruction.a_use_sp_imm1 << 32)
@@ -175,7 +172,6 @@ impl<'a> Emu<'a> {
         &mut self,
         instruction: &ZiskInst,
         emu_mem: &mut Vec<ZiskRequiredMemory>,
-        is_aligned: bool,
     ) {
         match instruction.b_src {
             SRC_C => self.ctx.inst_ctx.b = self.ctx.inst_ctx.c,
@@ -186,16 +182,14 @@ impl<'a> Emu<'a> {
                 }
                 self.ctx.inst_ctx.b = self.ctx.inst_ctx.mem.read(addr, 8);
 
-                if is_aligned == Self::is_8_aligned(addr, 8) {
-                    let required_memory = ZiskRequiredMemory {
-                        step: self.ctx.inst_ctx.step,
-                        is_write: false,
-                        address: addr,
-                        width: 8,
-                        value: self.ctx.inst_ctx.b,
-                    };
-                    emu_mem.push(required_memory);
-                }
+                let required_memory = ZiskRequiredMemory {
+                    step: self.ctx.inst_ctx.step,
+                    is_write: false,
+                    address: addr,
+                    width: 8,
+                    value: self.ctx.inst_ctx.b,
+                };
+                emu_mem.push(required_memory);
             }
             SRC_IMM => {
                 self.ctx.inst_ctx.b = instruction.b_offset_imm0 | (instruction.b_use_sp_imm1 << 32)
@@ -207,16 +201,14 @@ impl<'a> Emu<'a> {
                     addr += self.ctx.inst_ctx.sp;
                 }
                 self.ctx.inst_ctx.b = self.ctx.inst_ctx.mem.read(addr, instruction.ind_width);
-                if is_aligned == Self::is_8_aligned(addr, instruction.ind_width) {
-                    let required_memory = ZiskRequiredMemory {
-                        step: self.ctx.inst_ctx.step,
-                        is_write: false,
-                        address: addr,
-                        width: instruction.ind_width,
-                        value: self.ctx.inst_ctx.b,
-                    };
-                    emu_mem.push(required_memory);
-                }
+                let required_memory = ZiskRequiredMemory {
+                    step: self.ctx.inst_ctx.step,
+                    is_write: false,
+                    address: addr,
+                    width: instruction.ind_width,
+                    value: self.ctx.inst_ctx.b,
+                };
+                emu_mem.push(required_memory);
             }
             _ => panic!(
                 "Emu::source_b() Invalid b_src={} pc={}",
@@ -274,7 +266,6 @@ impl<'a> Emu<'a> {
         &mut self,
         instruction: &ZiskInst,
         emu_mem: &mut Vec<ZiskRequiredMemory>,
-        is_aligned: bool,
     ) {
         match instruction.store {
             STORE_NONE => {}
@@ -290,16 +281,14 @@ impl<'a> Emu<'a> {
                 }
                 self.ctx.inst_ctx.mem.write_silent(addr as u64, val as u64, 8);
 
-                if is_aligned == Self::is_8_aligned(addr as u64, 8) {
-                    let required_memory = ZiskRequiredMemory {
-                        step: self.ctx.inst_ctx.step,
-                        is_write: true,
-                        address: addr as u64,
-                        width: 8,
-                        value: val as u64,
-                    };
-                    emu_mem.push(required_memory);
-                }
+                let required_memory = ZiskRequiredMemory {
+                    step: self.ctx.inst_ctx.step,
+                    is_write: true,
+                    address: addr as u64,
+                    width: 8,
+                    value: val as u64,
+                };
+                emu_mem.push(required_memory);
             }
             STORE_IND => {
                 let val: i64 = if instruction.store_ra {
@@ -314,16 +303,14 @@ impl<'a> Emu<'a> {
                 addr += self.ctx.inst_ctx.a as i64;
                 self.ctx.inst_ctx.mem.write_silent(addr as u64, val as u64, instruction.ind_width);
 
-                if is_aligned == Self::is_8_aligned(addr as u64, instruction.ind_width) {
-                    let required_memory = ZiskRequiredMemory {
-                        step: self.ctx.inst_ctx.step,
-                        is_write: true,
-                        address: addr as u64,
-                        width: instruction.ind_width,
-                        value: val as u64,
-                    };
-                    emu_mem.push(required_memory);
-                }
+                let required_memory = ZiskRequiredMemory {
+                    step: self.ctx.inst_ctx.step,
+                    is_write: true,
+                    address: addr as u64,
+                    width: instruction.ind_width,
+                    value: val as u64,
+                };
+                emu_mem.push(required_memory);
             }
             _ => panic!(
                 "Emu::store_c() Invalid store={} pc={}",
@@ -615,11 +602,7 @@ impl<'a> Emu<'a> {
         (emu_traces, emu_segments)
     }
 
-    pub fn par_run_memory<F: PrimeField>(
-        &mut self,
-        inputs: Vec<u8>,
-        is_aligned: bool,
-    ) -> Vec<ZiskRequiredMemory> {
+    pub fn par_run_memory<F: PrimeField>(&mut self, inputs: Vec<u8>) -> Vec<ZiskRequiredMemory> {
         // Context, where the state of the execution is stored and modified at every execution step
         self.ctx = self.create_emu_context(inputs);
 
@@ -629,7 +612,7 @@ impl<'a> Emu<'a> {
         let mut emu_mem = Vec::new();
 
         while !self.ctx.inst_ctx.end {
-            self.par_step_memory::<F>(&mut emu_mem, is_aligned);
+            self.par_step_memory::<F>(&mut emu_mem);
         }
 
         emu_mem
@@ -811,27 +794,23 @@ impl<'a> Emu<'a> {
     /// Performs one single step of the emulation
     #[inline(always)]
     #[allow(unused_variables)]
-    pub fn par_step_memory<F: PrimeField>(
-        &mut self,
-        emu_mem: &mut Vec<ZiskRequiredMemory>,
-        is_aligned: bool,
-    ) {
+    pub fn par_step_memory<F: PrimeField>(&mut self, emu_mem: &mut Vec<ZiskRequiredMemory>) {
         let last_pc = self.ctx.inst_ctx.pc;
         let last_c = self.ctx.inst_ctx.c;
 
         let instruction = self.rom.get_instruction(self.ctx.inst_ctx.pc);
 
         // Build the 'a' register value  based on the source specified by the current instruction
-        self.source_a_memory(instruction, emu_mem, is_aligned);
+        self.source_a_memory(instruction, emu_mem);
 
         // Build the 'b' register value  based on the source specified by the current instruction
-        self.source_b_memory(instruction, emu_mem, is_aligned);
+        self.source_b_memory(instruction, emu_mem);
 
         // Call the operation
         (instruction.func)(&mut self.ctx.inst_ctx);
 
         // Store the 'c' register value based on the storage specified by the current instruction
-        self.store_c_memory(instruction, emu_mem, is_aligned);
+        self.store_c_memory(instruction, emu_mem);
 
         // Set SP, if specified by the current instruction
         // #[cfg(feature = "sp")]

emulator/src/emulator.rs

@@ -246,22 +246,15 @@ impl ZiskEmulator {
     pub fn par_process_rom_memory<F: PrimeField>(
         rom: &ZiskRom,
         inputs: &[u8],
-    ) -> Result<[Vec<ZiskRequiredMemory>; 2], ZiskEmulatorErr> {
-        let mut result: [Vec<ZiskRequiredMemory>; 2] = [Vec::new(), Vec::new()];
-
-        result.par_iter_mut().enumerate().for_each(|(is_aligned, result)| {
-            let is_aligned = is_aligned == 0;
-            let mut emu = Emu::new(rom);
-            let required = emu.par_run_memory::<F>(inputs.to_owned(), is_aligned);
-
-            if !emu.terminated() {
-                panic!("Emulation did not complete");
-                // TODO!
-                // return Err(ZiskEmulatorErr::EmulationNoCompleted);
-            }
+    ) -> Result<Vec<ZiskRequiredMemory>, ZiskEmulatorErr> {
+        let mut emu = Emu::new(rom);
+        let result = emu.par_run_memory::<F>(inputs.to_owned());
 
-            *result = required;
-        });
+        if !emu.terminated() {
+            panic!("Emulation did not complete");
+            // TODO!
+            // return Err(ZiskEmulatorErr::EmulationNoCompleted);
+        }
 
         Ok(result)
     }

pil/src/pil_helpers/pilout.rs

@@ -2,7 +2,7 @@
 // Manual modifications are not recommended and may be overwritten.
 use proofman_common::WitnessPilout;
 
-pub const PILOUT_HASH: &[u8] = b"ZiskMem-hash";
+pub const PILOUT_HASH: &[u8] = b"Zisk-hash";
 
 //AIRGROUP CONSTANTS
 
@@ -36,7 +36,7 @@ pub struct Pilout;
 
 impl Pilout {
     pub fn pilout() -> WitnessPilout {
-        let mut pilout = WitnessPilout::new("ZiskMem", 2, PILOUT_HASH.to_vec());
+        let mut pilout = WitnessPilout::new("Zisk", 2, PILOUT_HASH.to_vec());
 
         let air_group = pilout.add_air_group(Some("Zisk"));
 
@@ -54,4 +54,4 @@ impl Pilout {
 
         pilout
     }
-}
+}

pil/src/pil_helpers/traces.rs

@@ -45,4 +45,4 @@ trace!(SpecifiedRangesRow, SpecifiedRangesTrace<F> {
 
 trace!(U8AirRow, U8AirTrace<F> {
  mul: F,
-});
+});

state-machines/main/pil/main.pil

@@ -79,7 +79,7 @@ airtemplate Main(int N = 2**21, int RC = 2, int stack_enabled = 0, const int ope
         col witness air.b_imm1;
     }
     col witness b_src_ind;
-    col witness ind_width;  // 8 , 4, 2, 1
+    col witness ind_width;  // 8, 4, 2, 1
 
     // Operations related
 

state-machines/main/src/main_sm.rs

@@ -1,5 +1,6 @@
 use log::info;
 use p3_field::PrimeField;
+use sm_mem::MemProxy;
 
 use crate::InstanceExtensionCtx;
 use proofman_util::{timer_start_debug, timer_stop_and_log_debug};
@@ -26,6 +27,9 @@ pub struct MainSM<F: PrimeField> {
     /// Witness computation manager
     wcm: Arc<WitnessManager<F>>,
 
+    /// Memory state machine
+    mem_proxy_sm: Arc<MemProxy<F>>,
+
     /// Arithmetic state machine
     arith_sm: Arc<ArithSM>,
 
@@ -49,14 +53,16 @@ impl<F: PrimeField> MainSM<F> {
     /// * Arc to the MainSM state machine
     pub fn new(
         wcm: Arc<WitnessManager<F>>,
+        mem_proxy_sm: Arc<MemProxy<F>>,
         arith_sm: Arc<ArithSM>,
         binary_sm: Arc<BinarySM<F>>,
     ) -> Arc<Self> {
-        let main_sm = Arc::new(Self { wcm: wcm.clone(), arith_sm, binary_sm });
+        let main_sm = Arc::new(Self { wcm: wcm.clone(), mem_proxy_sm, arith_sm, binary_sm });
 
         wcm.register_component(main_sm.clone(), Some(ZISK_AIRGROUP_ID), Some(MAIN_AIR_IDS));
 
         // For all the secondary state machines, register the main state machine as a predecessor
+        main_sm.mem_proxy_sm.register_predecessor();
         main_sm.binary_sm.register_predecessor();
         main_sm.arith_sm.register_predecessor();
 

state-machines/mem/Cargo.toml

@@ -8,14 +8,19 @@ sm-common = { path = "../common" }
 zisk-core = { path = "../../core" }
 zisk-pil = { path = "../../pil" }
 
-p3-field = { workspace=true }
 proofman-common = { workspace = true }
 proofman-macros = { workspace = true }
 proofman-util = { workspace = true }
 proofman = { workspace = true }
+pil-std-lib = { workspace = true }
+
+p3-field = { workspace=true }
 log = { workspace = true }
 rayon = { workspace = true }
+num-bigint = { workspace = true }
+num-traits = "0.2"
 
 [features]
 default = []
-no_lib_link = ["proofman-common/no_lib_link", "proofman/no_lib_link"]
+no_lib_link = ["proofman-common/no_lib_link", "proofman/no_lib_link"]
+debug_mem_align = []

state-machines/mem/pil/mem.pil

@@ -53,7 +53,7 @@ airtemplate Mem(const int N = 2**21, const int RC = 2, const int id = MEMORY_ID,
 
     col witness first_addr_access_is_read;
     first_addr_access_is_read * (1 - first_addr_access_is_read) === 0;
-    (1 - first_addr_access_is_read) * rd * same_addr === 0;
+    (1 - first_addr_access_is_read) * rd * addr_changes === 0;
 
     for (int index = 0; index < length(value); index++) {
         same_value * (value[index] - 'value[index])  ===  0;