Merge pull request #39 from 0xPolygonHermez/emulator-performance

Improve emulator performance

core/src/zisk_definitions.rs

@@ -8,6 +8,7 @@ pub const STORE_NONE: u64 = 0;
 pub const STORE_MEM: u64 = 1;
 pub const STORE_IND: u64 = 2;
 pub const ROM_ADDR: u64 = 0x80000000;
+pub const ROM_ADDR_MAX: u64 = INPUT_ADDR - 1;
 pub const INPUT_ADDR: u64 = 0x90000000;
 pub const MAX_INPUT_SIZE: u64 = 0x10000000; // 256M,
 pub const RAM_ADDR: u64 = 0xa0000000;

core/src/zisk_inst.rs

@@ -26,6 +26,7 @@ pub struct ZiskInst {
     pub jmp_offset2: i64,
     pub is_external_op: bool,
     pub op: u8,
+    pub func: fn(u64, u64) -> (u64, bool),
     pub op_str: &'static str,
     pub verbose: String,
 }
@@ -55,6 +56,7 @@ impl Default for ZiskInst {
             jmp_offset2: 0,
             is_external_op: false,
             op: 0,
+            func: |_, _| (0, false),
             op_str: "",
             verbose: String::new(),
         }

core/src/zisk_inst_builder.rs

@@ -1,5 +1,5 @@
 use crate::{
-    ZiskInst, ZiskOperations, SRC_C, SRC_IMM, SRC_IND, SRC_MEM, SRC_SP, SRC_STEP, STORE_IND,
+    op_from_str, ZiskInst, SRC_C, SRC_IMM, SRC_IND, SRC_MEM, SRC_SP, SRC_STEP, STORE_IND,
     STORE_MEM, STORE_NONE, SYS_ADDR,
 };
 
@@ -11,13 +11,11 @@ const INITIAL_VALUE_S64: i64 = INVALID_VALUE_S64;
 pub struct ZiskInstBuilder {
     ind_width_set: bool,
     pub i: ZiskInst,
-    zisk_ops: ZiskOperations,
     regs_addr: u64,
 }
 
 impl ZiskInstBuilder {
     pub fn new(paddr: u64) -> ZiskInstBuilder {
-        let zisk_ops = ZiskOperations::new();
         let regs_addr = SYS_ADDR;
 
         ZiskInstBuilder {
@@ -43,10 +41,10 @@ impl ZiskInstBuilder {
                 jmp_offset2: INITIAL_VALUE_S64,
                 is_external_op: false,
                 op: 0,
+                func: |_, _| (0, false),
                 op_str: "",
                 verbose: String::new(),
             },
-            zisk_ops,
             regs_addr,
         }
     }
@@ -194,7 +192,7 @@ impl ZiskInstBuilder {
     }
 
     pub fn op(&mut self, optxt: &str) {
-        let op = self.zisk_ops.op_from_str.get(optxt).unwrap();
+        let op = op_from_str(optxt);
         if op.t == "i" {
             self.i.is_external_op = false;
         } else if op.t == "e" {
@@ -280,8 +278,10 @@ impl ZiskInstBuilder {
         self.i.verbose = s.to_owned();
     }
 
-    pub fn build(&self) {
+    pub fn build(&mut self) {
         //print!("ZiskInstBuilder::build() i=[ {} ]\n", self.i.to_string());
         self.check();
+
+        self.i.func = op_from_str(self.i.op_str).f;
     }
 }

core/src/zisk_operations.rs

@@ -573,3 +573,59 @@ impl ZiskOperations {
         ZiskOperations { ops, op_from_str, op_from_code }
     }
 }
+
+pub fn op_from_str(op: &str) -> ZiskOperation {
+    match op {
+        "flag" => ZiskOperation { n: "flag", t: "i", c: 0x00, f: op_flag },
+        "copyb" => ZiskOperation { n: "copyb", t: "i", c: 0x01, f: op_copyb },
+        "signextend_b" => ZiskOperation { n: "signextend_b", t: "e", c: 0x02, f: op_signextend_b },
+        "signextend_h" => ZiskOperation { n: "signextend_h", t: "e", c: 0x03, f: op_signextend_h },
+        "signextend_w" => ZiskOperation { n: "signextend_w", t: "e", c: 0x04, f: op_signextend_w },
+        "add" => ZiskOperation { n: "add", t: "e", c: 0x10, f: op_add },
+        "add_w" => ZiskOperation { n: "add_w", t: "e", c: 0x14, f: op_add_w },
+        "sub" => ZiskOperation { n: "sub", t: "e", c: 0x20, f: op_sub },
+        "sub_w" => ZiskOperation { n: "sub_w", t: "e", c: 0x24, f: op_sub_w },
+        "sll" => ZiskOperation { n: "sll", t: "e", c: 0x30, f: op_sll },
+        "sll_w" => ZiskOperation { n: "sll_w", t: "e", c: 0x34, f: op_sll_w },
+        "sra" => ZiskOperation { n: "sra", t: "e", c: 0x40, f: op_sra },
+        "srl" => ZiskOperation { n: "srl", t: "e", c: 0x41, f: op_srl },
+        "sra_w" => ZiskOperation { n: "sra_w", t: "e", c: 0x44, f: op_sra_w },
+        "srl_w" => ZiskOperation { n: "srl_w", t: "e", c: 0x45, f: op_srl_w },
+        "eq" => ZiskOperation { n: "eq", t: "e", c: 0x50, f: op_eq },
+        "eq_w" => ZiskOperation { n: "eq_w", t: "e", c: 0x54, f: op_eq_w },
+        "ltu" => ZiskOperation { n: "ltu", t: "e", c: 0x60, f: op_ltu },
+        "lt" => ZiskOperation { n: "lt", t: "e", c: 0x61, f: op_lt },
+        "ltu_w" => ZiskOperation { n: "ltu_w", t: "e", c: 0x64, f: op_ltu_w },
+        "lt_w" => ZiskOperation { n: "lt_w", t: "e", c: 0x65, f: op_lt_w },
+        "leu" => ZiskOperation { n: "leu", t: "e", c: 0x70, f: op_leu },
+        "le" => ZiskOperation { n: "le", t: "e", c: 0x71, f: op_le },
+        "leu_w" => ZiskOperation { n: "leu_w", t: "e", c: 0x74, f: op_leu_w },
+        "le_w" => ZiskOperation { n: "le_w", t: "e", c: 0x75, f: op_le_w },
+        "and" => ZiskOperation { n: "and", t: "e", c: 0x80, f: op_and },
+        "or" => ZiskOperation { n: "or", t: "e", c: 0x90, f: op_or },
+        "xor" => ZiskOperation { n: "xor", t: "e", c: 0xa0, f: op_xor },
+        "mulu" => ZiskOperation { n: "mulu", t: "e", c: 0xb0, f: op_mulu },
+        "mul" => ZiskOperation { n: "mul", t: "e", c: 0xb1, f: op_mul },
+        "mul_w" => ZiskOperation { n: "mul_w", t: "e", c: 0xb5, f: op_mul_w },
+        "muluh" => ZiskOperation { n: "muluh", t: "e", c: 0xb8, f: op_muluh },
+        "mulh" => ZiskOperation { n: "mulh", t: "e", c: 0xb9, f: op_mulh },
+        "mulsuh" => ZiskOperation { n: "mulsuh", t: "e", c: 0xbb, f: op_mulsuh },
+        "divu" => ZiskOperation { n: "divu", t: "e", c: 0xc0, f: op_divu },
+        "div" => ZiskOperation { n: "div", t: "e", c: 0xc1, f: op_div },
+        "divu_w" => ZiskOperation { n: "divu_w", t: "e", c: 0xc4, f: op_divu_w },
+        "div_w" => ZiskOperation { n: "div_w", t: "e", c: 0xc5, f: op_div_w },
+        "remu" => ZiskOperation { n: "remu", t: "e", c: 0xc8, f: op_remu },
+        "rem" => ZiskOperation { n: "rem", t: "e", c: 0xc9, f: op_rem },
+        "remu_w" => ZiskOperation { n: "remu_w", t: "e", c: 0xcc, f: op_remu_w },
+        "rem_w" => ZiskOperation { n: "rem_w", t: "e", c: 0xcd, f: op_rem_w },
+        "minu" => ZiskOperation { n: "minu", t: "e", c: 0xd0, f: op_minu },
+        "min" => ZiskOperation { n: "min", t: "e", c: 0xd1, f: op_min },
+        "minu_w" => ZiskOperation { n: "minu_w", t: "e", c: 0xd4, f: op_minu_w },
+        "min_w" => ZiskOperation { n: "min_w", t: "e", c: 0xd5, f: op_min_w },
+        "maxu" => ZiskOperation { n: "maxu", t: "e", c: 0xe0, f: op_maxu },
+        "max" => ZiskOperation { n: "max", t: "e", c: 0xe1, f: op_max },
+        "maxu_w" => ZiskOperation { n: "maxu_w", t: "e", c: 0xe4, f: op_maxu_w },
+        "max_w" => ZiskOperation { n: "max_w", t: "e", c: 0xe5, f: op_max_w },
+        _ => panic!("op_from_str() found invalid opcode={}", op),
+    }
+}

core/src/zisk_rom.rs

@@ -1,7 +1,7 @@
 use std::collections::HashMap;
 
-use crate::{ZiskInst, INVALID_VALUE}; /* TODO: Ask Jordi.  b_offset_imm0 is signed, so it could easily
-                                       * become 0xFFFFFFFFFFFFFFFF */
+use crate::{ZiskInst, INVALID_VALUE, ROM_ADDR, ROM_ENTRY}; /* TODO: Ask Jordi.  b_offset_imm0 is signed, so it could easily
+                                                            * become 0xFFFFFFFFFFFFFFFF */
 use crate::{ZiskInstBuilder, INVALID_VALUE_S64, SRC_IND, SRC_SP, SRC_STEP}; // TODO: Ask Jordi.
 
 /// RO data structure
@@ -31,6 +31,9 @@ pub struct ZiskRom {
     pub data: Vec<u8>,
     pub rom_entry_instructions: Vec<ZiskInst>,
     pub rom_instructions: Vec<ZiskInst>,
+    // Rom Non 4 bytes aligned instructions
+    pub offset_rom_na_unstructions: u64,
+    pub rom_na_instructions: Vec<ZiskInst>,
 }
 
 /// ZisK ROM implementation
@@ -45,6 +48,26 @@ impl ZiskRom {
             data: Vec::new(),
             rom_entry_instructions: Vec::new(),
             rom_instructions: Vec::new(),
+            offset_rom_na_unstructions: 0,
+            rom_na_instructions: Vec::new(),
+        }
+    }
+
+    #[inline(always)]
+    pub fn get_instruction(&self, pc: u64) -> &ZiskInst {
+        if pc >= ROM_ADDR {
+            if pc & 0b11 == 0 {
+                // pc is aligned to a 4-byte boundary
+                &self.rom_instructions[((pc - ROM_ADDR) >> 2) as usize]
+            } else {
+                // pc is not aligned to a 4-byte boundary
+                &self.rom_na_instructions[(pc - self.offset_rom_na_unstructions) as usize]
+            }
+        } else if pc >= ROM_ENTRY {
+            // pc is in the ROM_ENTRY range
+            &self.rom_entry_instructions[((pc - ROM_ENTRY) >> 2) as usize]
+        } else {
+            panic!("ZiskRom::get_instruction() pc={} is out of range", pc);
         }
     }
 

emulator/benches/benchmark.rs

@@ -52,11 +52,8 @@ fn bench_emulate(c: &mut Criterion) {
     c.bench_function("Emulate", |b| {
         b.iter(|| {
             let options = EmuOptions {
-                elf: Some(
-                    "/Users/xpinsach/dev/zisk/emulator/../../tmp/zisk-fibonacci/target/riscv64ima-polygon-ziskos-elf/release/fibonacci"
-                        .to_string(),
-                ),
-                inputs: Some("/Users/xpinsach/dev/zisk/emulator/../../tmp/zisk-fibonacci/output/input.bin".to_string()),
+                elf: Some("./benches/data/my.elf".to_string()),
+                inputs: Some("./benches/data/input.bin".to_string()),
                 log_metrics: true,
                 ..Default::default()
             };

emulator/src/emu.rs

@@ -3,8 +3,8 @@ use std::mem;
 use crate::{EmuContext, EmuOptions, EmuTrace, MemTrace};
 use riscv::RiscVRegisters;
 use zisk_core::{
-    opcode_execute, ZiskRom, OUTPUT_ADDR, ROM_ADDR, ROM_ENTRY, SRC_C, SRC_IMM, SRC_IND, SRC_MEM,
-    SRC_SP, SRC_STEP, STORE_IND, STORE_MEM, STORE_NONE, SYS_ADDR,
+    ZiskRom, OUTPUT_ADDR, SRC_C, SRC_IMM, SRC_IND, SRC_MEM, SRC_SP, SRC_STEP, STORE_IND, STORE_MEM,
+    STORE_NONE, SYS_ADDR,
 };
 
 /// ZisK emulator structure, containing the ZisK rom, the list of ZisK operations, and the
@@ -52,22 +52,15 @@ impl<'a> Emu<'a> {
             self.ctx.mem_trace.clear();
         }
 
-        let instruction = if self.ctx.pc >= ROM_ADDR {
-            &self.rom.rom_instructions[(self.ctx.pc - ROM_ADDR) as usize]
-        } else if self.ctx.pc >= ROM_ENTRY {
-            &self.rom.rom_entry_instructions[(self.ctx.pc - ROM_ENTRY) as usize]
-        } else {
-            self.ctx.end = true;
-            return;
-        };
+        let pc = self.ctx.pc;
+
+        let instruction = self.rom.get_instruction(pc);
 
         //println!("Emu::step() executing step={} pc={:x} inst={}", ctx.step, ctx.pc,
         // inst.i.to_string()); println!("Emu::step() step={} pc={}", ctx.step, ctx.pc);
 
         // If this is the last instruction, stop executing
-        if instruction.end {
-            self.ctx.end = true;
-        }
+        self.ctx.end = instruction.end;
 
         // Build the 'a' register value  based on the source specified by the current instruction
         match instruction.a_src {
@@ -111,8 +104,9 @@ impl<'a> Emu<'a> {
             }
             _ => panic!("Emu::step() Invalid b_src={} pc={}", instruction.b_src, self.ctx.pc),
         }
+
         // Call the operation
-        (self.ctx.c, self.ctx.flag) = opcode_execute(instruction.op, self.ctx.a, self.ctx.b);
+        (self.ctx.c, self.ctx.flag) = (instruction.func)(self.ctx.a, self.ctx.b);
 
         // Store the 'c' register value based on the storage specified by the current instruction
         match instruction.store {
@@ -129,8 +123,7 @@ impl<'a> Emu<'a> {
                 }
                 self.ctx.mem.write(addr as u64, val as u64, 8);
                 if tracing_steps {
-                    let mem_trace = MemTrace::new(true, addr as u64, 8, val as u64);
-                    self.ctx.mem_trace.push(mem_trace);
+                    self.ctx.mem_trace.push(MemTrace::new(true, addr as u64, 8, val as u64));
                 }
             }
             STORE_IND => {
@@ -273,7 +266,7 @@ impl<'a> Emu<'a> {
                 println!("Emu::run() step={} ctx.pc={}", self.ctx.step, self.ctx.pc);
             }
             // Check trace PC
-            if self.ctx.tracerv_on && (self.ctx.pc % 4 == 0) {
+            if self.ctx.tracerv_on && (self.ctx.pc & 0b11 == 0) {
                 self.ctx.trace_pc = self.ctx.pc;
             }
 
@@ -294,7 +287,7 @@ impl<'a> Emu<'a> {
             self.step(options, &callback);
 
             // Only trace after finishing a riscV instruction
-            if self.ctx.tracerv_on && ((self.ctx.pc % 4) == 0) {
+            if self.ctx.tracerv_on && (self.ctx.pc & 0b11) == 0 {
                 // Store logs in a vector of strings
                 let mut changes: Vec<String> = Vec::new();
 

emulator/src/emulator.rs

@@ -4,7 +4,7 @@ use std::{
     path::{Path, PathBuf},
     time::Instant,
 };
-use zisk_core::{Riscv2zisk, ZiskInst, ZiskRom, INPUT_ADDR, ROM_ADDR, ROM_ENTRY};
+use zisk_core::{Riscv2zisk, ZiskInst, ZiskRom, ROM_ADDR, ROM_ADDR_MAX, ROM_ENTRY};
 
 pub trait Emulator<ET> {
     fn emulate(
@@ -87,39 +87,68 @@ impl ZiskEmulator {
         // Preprocess the ROM (experimental)
         let mut max_rom_entry = 0;
         let mut max_rom_instructions = 0;
+
+        let mut min_rom_na_unstructions = u64::MAX;
+        let mut max_rom_na_unstructions = 0;
         for instruction in &rom.insts {
             let addr = *instruction.0;
 
             if addr < ROM_ENTRY {
                 return Err(ZiskEmulatorErr::AddressOutOfRange(addr));
             } else if addr < ROM_ADDR {
-                max_rom_entry = std::cmp::max(max_rom_entry, addr);
-            } else if addr < INPUT_ADDR {
-                max_rom_instructions = max_rom_instructions.max(addr);
+                if addr % 4 != 0 {
+                    // When an address is not 4 bytes aligned, it is considered a
+                    // na_rom_instructions We are supposed to have only one non
+                    // aligned instructions in > ROM_ADDRESS
+                    min_rom_na_unstructions = std::cmp::min(min_rom_na_unstructions, addr);
+                    max_rom_na_unstructions = std::cmp::max(max_rom_na_unstructions, addr);
+                } else {
+                    max_rom_entry = std::cmp::max(max_rom_entry, addr);
+                }
+            } else if addr < ROM_ADDR_MAX {
+                if addr % 4 != 0 {
+                    // When an address is not 4 bytes aligned, it is considered a
+                    // na_rom_instructions We are supposed to have only one non
+                    // aligned instructions in > ROM_ADDRESS
+                    min_rom_na_unstructions = std::cmp::min(min_rom_na_unstructions, addr);
+                    max_rom_na_unstructions = std::cmp::max(max_rom_na_unstructions, addr);
+                } else {
+                    max_rom_instructions = max_rom_instructions.max(addr);
+                }
             } else {
                 return Err(ZiskEmulatorErr::AddressOutOfRange(addr));
             }
         }
 
-        let num_rom_entry = max_rom_entry - ROM_ENTRY + 1;
-        let num_rom_instructions = max_rom_instructions - ROM_ADDR + 1;
+        let num_rom_entry = (max_rom_entry - ROM_ENTRY) / 4 + 1;
+        let num_rom_instructions = (max_rom_instructions - ROM_ADDR) / 4 + 1;
+        let num_rom_na_instructions = if u64::MAX == min_rom_na_unstructions {
+            0
+        } else {
+            max_rom_na_unstructions - min_rom_na_unstructions + 1
+        };
 
         rom.rom_entry_instructions = vec![ZiskInst::default(); num_rom_entry as usize];
         rom.rom_instructions = vec![ZiskInst::default(); num_rom_instructions as usize];
+        rom.rom_na_instructions = vec![ZiskInst::default(); num_rom_na_instructions as usize];
+        rom.offset_rom_na_unstructions = min_rom_na_unstructions;
 
         for instruction in &rom.insts {
             let addr = *instruction.0;
 
-            if addr < ROM_ADDR {
-                rom.rom_entry_instructions[(addr - ROM_ENTRY) as usize] = instruction.1.i.clone();
+            if addr % 4 != 0 {
+                rom.rom_na_instructions[(addr - min_rom_na_unstructions) as usize] =
+                    instruction.1.i.clone();
+            } else if addr < ROM_ADDR {
+                rom.rom_entry_instructions[((addr - ROM_ENTRY) >> 2) as usize] =
+                    instruction.1.i.clone();
             } else {
-                rom.rom_instructions[(addr - ROM_ADDR) as usize] = instruction.1.i.clone();
+                rom.rom_instructions[((addr - ROM_ADDR) >> 2) as usize] = instruction.1.i.clone();
             }
         }
 
         // Create a emulator instance with this rom and inputs
         let mut emu = Emu::new(rom);
-
         let start = Instant::now();
 
         // Run the emulation

emulator/src/mem.rs

@@ -1,5 +1,4 @@
 use crate::MemSection;
-use zisk_core::UART_ADDR;
 
 /// Memory structure, containing several read sections and one single write section
 pub struct Mem {
@@ -153,8 +152,8 @@ impl Mem {
         }
 
         // Log to console bytes written to UART address
-        if (addr == UART_ADDR) && (width == 1) {
-            print!("{}", String::from(val as u8 as char));
-        }
+        // if (addr == UART_ADDR) && (width == 1) {
+        //     print!("{}", String::from(val as u8 as char));
+        // }
     }
 }