Merge pull request #230 from mooori/compile-flags

feat: add ISA specific setting `instrument_inst` for zkASM

cranelift/codegen/meta/src/isa/zkasm.rs

@@ -2,6 +2,16 @@ use crate::cdsl::isa::TargetIsa;
 use crate::cdsl::settings::SettingGroupBuilder;
 
 pub(crate) fn define() -> TargetIsa {
-    let setting = SettingGroupBuilder::new("zkasm");
-    TargetIsa::new("zkasm", setting.build())
+    let mut settings = SettingGroupBuilder::new("zkasm");
+
+    // Benchmarking
+    settings.add_bool(
+        "emit_profiling_info",
+        "Instrument `Inst` to identify hot instructions.",
+        "Inserts calls to `zkevm-proverjs` helpers that trace `Inst`s \
+        executed at runtime",
+        false,
+    );
+
+    TargetIsa::new("zkasm", settings.build())
 }

cranelift/codegen/src/isa/zkasm/inst/emit.rs

@@ -367,6 +367,16 @@ impl Inst {
         }
         insts */
     }
+
+    /// Emits zkASM to trace executed instructions.
+    fn emit_inst_instrumentation(&self, sink: &mut MachBuffer<Inst>) {
+        match self {
+            // Labels are handled separately since benchmarking will provide a separate command to
+            // analyze labels.
+            &MInst::Label { .. } => {}
+            _ => put_string(";; TODO(mooori) add instruction instrumentation\n", sink),
+        }
+    }
 }
 
 fn put_string(s: &str, sink: &mut MachBuffer<Inst>) {
@@ -411,6 +421,13 @@ impl MachInstEmit for Inst {
         // to allow disabling the check for `JTSequence`, which is always
         // emitted following an `EmitIsland`.
         let mut start_off = sink.cur_offset();
+
+        if emit_info.isa_flags.emit_profiling_info() {
+            // Emit instrumentation *before* the instruction's zkASM. Otherwise
+            // instrumentation might be skipped, e.g. in case of jumps.
+            self.emit_inst_instrumentation(sink);
+        }
+
         match self {
             &Inst::Nop => {}
             &Inst::Label { imm } => {

cranelift/codegen/src/isa/zkasm/mod.rs

@@ -18,7 +18,7 @@ use target_lexicon::{Architecture, Triple};
 mod abi;
 pub(crate) mod inst;
 mod lower;
-mod settings;
+pub mod settings;
 
 use self::inst::EmitInfo;
 

cranelift/filetests/src/test_zkasm.rs

@@ -1,6 +1,7 @@
 #[cfg(test)]
 mod tests {
     use crate::zkasm_codegen;
+    use crate::zkasm_codegen::ZkasmSettings;
     use std::collections::HashMap;
     use std::path::{Path, PathBuf};
 
@@ -59,7 +60,8 @@ mod tests {
 
     fn test_module(name: &str) {
         let module_binary = wat::parse_file(format!("../zkasm_data/{name}.wat")).unwrap();
-        let program = zkasm_codegen::generate_zkasm(&module_binary);
+        let settings = ZkasmSettings::default();
+        let program = zkasm_codegen::generate_zkasm(&settings, &module_binary);
         let expected =
             expect_test::expect_file![format!("../../zkasm_data/generated/{name}.zkasm")];
         expected.assert_eq(&program);
@@ -151,7 +153,8 @@ mod tests {
                 .join(path)
                 .join(format!("generated/{name}.zkasm"))];
             let result = std::panic::catch_unwind(|| {
-                let program = zkasm_codegen::generate_zkasm(&module_binary);
+                let settings = ZkasmSettings::default();
+                let program = zkasm_codegen::generate_zkasm(&settings, &module_binary);
                 expected.assert_eq(&program);
             });
             if let Err(err) = result {

cranelift/filetests/src/zkasm_codegen.rs

@@ -1,19 +1,30 @@
+use std::collections::HashMap;
+use std::sync::Arc;
+
 use cranelift_codegen::data_value::DataValue;
 use cranelift_codegen::entity::EntityRef;
 use cranelift_codegen::ir::function::FunctionParameters;
 use cranelift_codegen::ir::ExternalName;
 use cranelift_codegen::ir::Function;
-use cranelift_codegen::isa::zkasm;
-use cranelift_codegen::{settings, FinalizedMachReloc, FinalizedRelocTarget};
+use cranelift_codegen::isa::{zkasm, IsaBuilder, TargetIsa};
+use cranelift_codegen::settings::Configurable;
+use cranelift_codegen::{settings, CodegenError, FinalizedMachReloc, FinalizedRelocTarget};
 use cranelift_reader::Comparison;
 use cranelift_reader::Invocation;
 use cranelift_wasm::{translate_module, ZkasmEnvironment};
-use std::collections::HashMap;
+
+/// ISA specific settings for zkASM codegen.
+#[derive(Default, Debug)]
+pub struct ZkasmSettings {
+    /// Instruments generated zkASM to trace executed instructions.
+    pub emit_profiling_info: bool,
+}
 
 #[allow(dead_code)]
-pub fn generate_zkasm(wasm_module: &[u8]) -> String {
+pub fn generate_zkasm(settings: &ZkasmSettings, wasm_module: &[u8]) -> String {
     let flag_builder = settings::builder();
-    let isa_builder = zkasm::isa_builder("zkasm-unknown-unknown".parse().unwrap());
+    let mut isa_builder = zkasm::isa_builder("zkasm-unknown-unknown".parse().unwrap());
+    handle_zkasm_settings(settings, &mut isa_builder);
     let isa = isa_builder
         .finish(settings::Flags::new(flag_builder))
         .unwrap();
@@ -63,6 +74,15 @@ pub fn generate_zkasm(wasm_module: &[u8]) -> String {
     program.join("\n")
 }
 
+fn handle_zkasm_settings(
+    settings: &ZkasmSettings,
+    isa_builder: &mut IsaBuilder<Result<Arc<dyn TargetIsa>, CodegenError>>,
+) {
+    if settings.emit_profiling_info {
+        isa_builder.enable("emit_profiling_info").unwrap();
+    }
+}
+
 #[allow(dead_code)]
 pub fn generate_preamble(
     start_func_index: usize,