feat(tooling): Skip program transformation when loaded from cache (#6689)

aakoshh · web-flow · commit 7feb6589d982 · 2024-12-03T15:01:10.000Z
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/acvm-repo/acir/src/circuit/brillig.rs b/acvm-repo/acir/src/circuit/brillig.rs
@@ -5,7 +5,7 @@ use serde::{Deserialize, Serialize};
 
 /// Inputs for the Brillig VM. These are the initial inputs
 /// that the Brillig VM will use to start.
-#[derive(Clone, PartialEq, Eq, Serialize, Deserialize, Debug)]
+#[derive(Clone, PartialEq, Eq, Serialize, Deserialize, Debug, Hash)]
 pub enum BrilligInputs<F> {
     Single(Expression<F>),
     Array(Vec<Expression<F>>),
@@ -14,7 +14,7 @@ pub enum BrilligInputs<F> {
 
 /// Outputs for the Brillig VM. Once the VM has completed
 /// execution, this will be the object that is returned.
-#[derive(Clone, PartialEq, Eq, Serialize, Deserialize, Debug)]
+#[derive(Clone, PartialEq, Eq, Serialize, Deserialize, Debug, Hash)]
 pub enum BrilligOutputs {
     Simple(Witness),
     Array(Vec<Witness>),
@@ -23,7 +23,7 @@ pub enum BrilligOutputs {
 /// This is purely a wrapper struct around a list of Brillig opcode's which represents
 /// a full Brillig function to be executed by the Brillig VM.
 /// This is stored separately on a program and accessed through a [BrilligPointer].
-#[derive(Clone, PartialEq, Eq, Serialize, Deserialize, Default, Debug)]
+#[derive(Clone, PartialEq, Eq, Serialize, Deserialize, Default, Debug, Hash)]
 pub struct BrilligBytecode<F> {
     pub bytecode: Vec<BrilligOpcode<F>>,
 }
diff --git a/acvm-repo/acir/src/circuit/mod.rs b/acvm-repo/acir/src/circuit/mod.rs
@@ -25,7 +25,7 @@ use self::{brillig::BrilligBytecode, opcodes::BlockId};
 /// Bounded Expressions are useful if you are eventually going to pass the ACIR
 /// into a proving system which supports PLONK, where arithmetic expressions have a
 /// finite fan-in.
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Default)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Default, Hash)]
 pub enum ExpressionWidth {
     #[default]
     Unbounded,
@@ -36,13 +36,13 @@ pub enum ExpressionWidth {
 
 /// A program represented by multiple ACIR circuits. The execution trace of these
 /// circuits is dictated by construction of the [crate::native_types::WitnessStack].
-#[derive(Clone, PartialEq, Eq, Serialize, Deserialize, Default)]
+#[derive(Clone, PartialEq, Eq, Serialize, Deserialize, Default, Hash)]
 pub struct Program<F> {
     pub functions: Vec<Circuit<F>>,
     pub unconstrained_functions: Vec<BrilligBytecode<F>>,
 }
 
-#[derive(Clone, PartialEq, Eq, Serialize, Deserialize, Default)]
+#[derive(Clone, PartialEq, Eq, Serialize, Deserialize, Default, Hash)]
 pub struct Circuit<F> {
     // current_witness_index is the highest witness index in the circuit. The next witness to be added to this circuit
     // will take on this value. (The value is cached here as an optimization.)
@@ -69,13 +69,13 @@ pub struct Circuit<F> {
     pub assert_messages: Vec<(OpcodeLocation, AssertionPayload<F>)>,
 }
 
-#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Hash)]
 pub enum ExpressionOrMemory<F> {
     Expression(Expression<F>),
     Memory(BlockId),
 }
 
-#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Hash)]
 pub struct AssertionPayload<F> {
     pub error_selector: u64,
     pub payload: Vec<ExpressionOrMemory<F>>,
@@ -355,7 +355,7 @@ impl<F: AcirField> std::fmt::Debug for Program<F> {
     }
 }
 
-#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize, Default)]
+#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize, Default, Hash)]
 pub struct PublicInputs(pub BTreeSet<Witness>);
 
 impl PublicInputs {
diff --git a/acvm-repo/acir/src/circuit/opcodes.rs b/acvm-repo/acir/src/circuit/opcodes.rs
@@ -15,7 +15,7 @@ pub use black_box_function_call::{
 };
 pub use memory_operation::{BlockId, MemOp};
 
-#[derive(Clone, PartialEq, Eq, Serialize, Deserialize)]
+#[derive(Clone, PartialEq, Eq, Serialize, Deserialize, Hash)]
 pub enum BlockType {
     Memory,
     CallData(u32),
@@ -29,7 +29,7 @@ impl BlockType {
 }
 
 #[allow(clippy::large_enum_variant)]
-#[derive(Clone, PartialEq, Eq, Serialize, Deserialize)]
+#[derive(Clone, PartialEq, Eq, Serialize, Deserialize, Hash)]
 pub enum Opcode<F> {
     /// An `AssertZero` opcode adds the constraint that `P(w) = 0`, where
     /// `w=(w_1,..w_n)` is a tuple of `n` witnesses, and `P` is a multi-variate
diff --git a/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs b/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs
@@ -9,13 +9,13 @@ use thiserror::Error;
 // Note: Some functions will not use all of the witness
 // So we need to supply how many bits of the witness is needed
 
-#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)]
+#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize, Hash)]
 pub enum ConstantOrWitnessEnum<F> {
     Constant(F),
     Witness(Witness),
 }
 
-#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)]
+#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize, Hash)]
 pub struct FunctionInput<F> {
     input: ConstantOrWitnessEnum<F>,
     num_bits: u32,
@@ -79,7 +79,7 @@ impl<F: std::fmt::Display> std::fmt::Display for FunctionInput<F> {
     }
 }
 
-#[derive(Clone, PartialEq, Eq, Serialize, Deserialize)]
+#[derive(Clone, PartialEq, Eq, Serialize, Deserialize, Hash)]
 pub enum BlackBoxFuncCall<F> {
     AES128Encrypt {
         inputs: Vec<FunctionInput<F>>,
diff --git a/acvm-repo/acir/src/circuit/opcodes/memory_operation.rs b/acvm-repo/acir/src/circuit/opcodes/memory_operation.rs
@@ -7,7 +7,7 @@ pub struct BlockId(pub u32);
 
 /// Operation on a block of memory
 /// We can either write or read at an index in memory
-#[derive(Clone, PartialEq, Eq, Serialize, Deserialize, Debug)]
+#[derive(Clone, PartialEq, Eq, Serialize, Deserialize, Debug, Hash)]
 pub struct MemOp<F> {
     /// A constant expression that can be 0 (read) or 1 (write)
     pub operation: Expression<F>,
diff --git a/acvm-repo/brillig/src/black_box.rs b/acvm-repo/brillig/src/black_box.rs
@@ -3,7 +3,7 @@ use serde::{Deserialize, Serialize};
 
 /// These opcodes provide an equivalent of ACIR blackbox functions.
 /// They are implemented as native functions in the VM.
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Hash)]
 pub enum BlackBoxOp {
     /// Encrypts a message using AES128.
     AES128Encrypt {
diff --git a/acvm-repo/brillig/src/opcodes.rs b/acvm-repo/brillig/src/opcodes.rs
@@ -56,7 +56,7 @@ impl MemoryAddress {
 }
 
 /// Describes the memory layout for an array/vector element
-#[derive(Debug, Clone, Eq, PartialEq, Serialize, Deserialize)]
+#[derive(Debug, Clone, Eq, PartialEq, Serialize, Deserialize, Hash)]
 pub enum HeapValueType {
     // A single field element is enough to represent the value with a given bit size
     Simple(BitSize),
@@ -81,7 +81,7 @@ impl HeapValueType {
 }
 
 /// A fixed-sized array starting from a Brillig memory location.
-#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Copy)]
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Copy, Hash)]
 pub struct HeapArray {
     pub pointer: MemoryAddress,
     pub size: usize,
@@ -94,13 +94,13 @@ impl Default for HeapArray {
 }
 
 /// A memory-sized vector passed starting from a Brillig memory location and with a memory-held size
-#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Copy)]
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Copy, Hash)]
 pub struct HeapVector {
     pub pointer: MemoryAddress,
     pub size: MemoryAddress,
 }
 
-#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Copy, PartialOrd, Ord)]
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Copy, PartialOrd, Ord, Hash)]
 pub enum IntegerBitSize {
     U1,
     U8,
@@ -152,7 +152,7 @@ impl std::fmt::Display for IntegerBitSize {
     }
 }
 
-#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Copy, PartialOrd, Ord)]
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Copy, PartialOrd, Ord, Hash)]
 pub enum BitSize {
     Field,
     Integer(IntegerBitSize),
@@ -181,7 +181,7 @@ impl BitSize {
 /// While we are usually agnostic to how memory is passed within Brillig,
 /// this needs to be encoded somehow when dealing with an external system.
 /// For simplicity, the extra type information is given right in the ForeignCall instructions.
-#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Copy)]
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Copy, Hash)]
 pub enum ValueOrArray {
     /// A single value passed to or from an external call
     /// It is an 'immediate' value - used without dereferencing.
@@ -198,7 +198,7 @@ pub enum ValueOrArray {
     HeapVector(HeapVector),
 }
 
-#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Hash)]
 pub enum BrilligOpcode<F> {
     /// Takes the fields in addresses `lhs` and `rhs`
     /// Performs the specified binary operation
@@ -314,7 +314,7 @@ pub enum BrilligOpcode<F> {
 }
 
 /// Binary fixed-length field expressions
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Hash)]
 pub enum BinaryFieldOp {
     Add,
     Sub,
@@ -332,7 +332,7 @@ pub enum BinaryFieldOp {
 }
 
 /// Binary fixed-length integer expressions
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Hash)]
 pub enum BinaryIntOp {
     Add,
     Sub,
diff --git a/compiler/noirc_driver/src/debug.rs b/compiler/noirc_driver/src/debug.rs
@@ -8,7 +8,7 @@ use std::{
 
 /// For a given file, we store the source code and the path to the file
 /// so consumers of the debug artifact can reconstruct the original source code structure.
-#[derive(Clone, Debug, Serialize, Deserialize)]
+#[derive(Clone, Debug, Serialize, Deserialize, Hash)]
 pub struct DebugFile {
     pub source: String,
     pub path: PathBuf,
diff --git a/compiler/noirc_driver/src/lib.rs b/compiler/noirc_driver/src/lib.rs
@@ -334,6 +334,8 @@ pub fn compute_function_abi(
 ///
 /// On success this returns the compiled program alongside any warnings that were found.
 /// On error this returns the non-empty list of warnings and errors.
+///
+/// See [compile_no_check] for further information about the use of `cached_program`.
 pub fn compile_main(
     context: &mut Context,
     crate_id: CrateId,
@@ -555,6 +557,15 @@ pub const DEFAULT_EXPRESSION_WIDTH: ExpressionWidth = ExpressionWidth::Bounded {
 /// Compile the current crate using `main_function` as the entrypoint.
 ///
 /// This function assumes [`check_crate`] is called beforehand.
+///
+/// If the program is not returned from cache, it is backend-agnostic and must go through a transformation
+/// pass before usage in proof generation; if it's returned from cache these transformations might have
+/// already been applied.
+///
+/// The transformations are _not_ covered by the check that decides whether we can use the cached artifact.
+/// That comparison is based on on [CompiledProgram::hash] which is a persisted version of the hash of the input
+/// [`ast::Program`][noirc_frontend::monomorphization::ast::Program], whereas the output [`circuit::Program`][acir::circuit::Program]
+/// contains the final optimized ACIR opcodes, including the transformation done after this compilation.
 #[tracing::instrument(level = "trace", skip_all, fields(function_name = context.function_name(&main_function)))]
 pub fn compile_no_check(
     context: &mut Context,
@@ -569,8 +580,6 @@ pub fn compile_no_check(
         monomorphize(main_function, &mut context.def_interner)?
     };
 
-    let hash = fxhash::hash64(&program);
-    let hashes_match = cached_program.as_ref().map_or(false, |program| program.hash == hash);
     if options.show_monomorphized {
         println!("{program}");
     }
@@ -584,10 +593,16 @@ pub fn compile_no_check(
         || options.show_ssa
         || options.emit_ssa;
 
-    if !force_compile && hashes_match {
-        info!("Program matches existing artifact, returning early");
-        return Ok(cached_program.expect("cache must exist for hashes to match"));
+    // Hash the AST program, which is going to be used to fingerprint the compilation artifact.
+    let hash = fxhash::hash64(&program);
+
+    if let Some(cached_program) = cached_program {
+        if !force_compile && cached_program.hash == hash {
+            info!("Program matches existing artifact, returning early");
+            return Ok(cached_program);
+        }
     }
+
     let return_visibility = program.return_visibility;
     let ssa_evaluator_options = noirc_evaluator::ssa::SsaEvaluatorOptions {
         ssa_logging: match &options.show_ssa_pass_name {
diff --git a/compiler/noirc_driver/src/program.rs b/compiler/noirc_driver/src/program.rs
@@ -9,7 +9,7 @@ use serde::{Deserialize, Serialize};
 
 use super::debug::DebugFile;
 
-#[derive(Debug, Serialize, Deserialize, Clone)]
+#[derive(Debug, Serialize, Deserialize, Clone, Hash)]
 pub struct CompiledProgram {
     pub noir_version: String,
     /// Hash of the [`Program`][noirc_frontend::monomorphization::ast::Program] from which this [`CompiledProgram`]
diff --git a/compiler/noirc_errors/src/debug_info.rs b/compiler/noirc_errors/src/debug_info.rs
@@ -94,7 +94,7 @@ impl ProgramDebugInfo {
 }
 
 #[serde_as]
-#[derive(Default, Debug, Clone, Deserialize, Serialize)]
+#[derive(Default, Debug, Clone, Deserialize, Serialize, Hash)]
 pub struct DebugInfo {
     /// Map opcode index of an ACIR circuit into the source code location
     /// Serde does not support mapping keys being enums for json, so we indicate
diff --git a/compiler/noirc_evaluator/src/errors.rs b/compiler/noirc_evaluator/src/errors.rs
@@ -63,7 +63,7 @@ pub enum RuntimeError {
     UnknownReference { call_stack: CallStack },
 }
 
-#[derive(Debug, Clone, Serialize, Deserialize)]
+#[derive(Debug, Clone, Serialize, Deserialize, Hash)]
 pub enum SsaReport {
     Warning(InternalWarning),
     Bug(InternalBug),
@@ -107,15 +107,15 @@ impl From<SsaReport> for FileDiagnostic {
     }
 }
 
-#[derive(Debug, PartialEq, Eq, Clone, Error, Serialize, Deserialize)]
+#[derive(Debug, PartialEq, Eq, Clone, Error, Serialize, Deserialize, Hash)]
 pub enum InternalWarning {
     #[error("Return variable contains a constant value")]
     ReturnConstant { call_stack: CallStack },
     #[error("Calling std::verify_proof(...) does not verify a proof")]
     VerifyProof { call_stack: CallStack },
 }
 
-#[derive(Debug, PartialEq, Eq, Clone, Error, Serialize, Deserialize)]
+#[derive(Debug, PartialEq, Eq, Clone, Error, Serialize, Deserialize, Hash)]
 pub enum InternalBug {
     #[error("Input to brillig function is in a separate subgraph to output")]
     IndependentSubgraph { call_stack: CallStack },
diff --git a/compiler/noirc_evaluator/src/ssa.rs b/compiler/noirc_evaluator/src/ssa.rs
@@ -238,7 +238,7 @@ impl SsaProgramArtifact {
     }
 }
 
-/// Compiles the [`Program`] into [`ACIR``][acvm::acir::circuit::Program].
+/// Compiles the [`Program`] into [`ACIR`][acvm::acir::circuit::Program].
 ///
 /// The output ACIR is backend-agnostic and so must go through a transformation pass before usage in proof generation.
 #[tracing::instrument(level = "trace", skip_all)]
diff --git a/tooling/nargo/src/ops/check.rs b/tooling/nargo/src/ops/check.rs
@@ -2,8 +2,8 @@ use acvm::compiler::CircuitSimulator;
 use noirc_driver::{CompiledProgram, ErrorsAndWarnings};
 use noirc_errors::{CustomDiagnostic, FileDiagnostic};
 
+/// Run each function through a circuit simulator to check that they are solvable.
 pub fn check_program(compiled_program: &CompiledProgram) -> Result<(), ErrorsAndWarnings> {
-    // Check if the program is solvable
     for (i, circuit) in compiled_program.program.functions.iter().enumerate() {
         let mut simulator = CircuitSimulator::default();
         if !simulator.check_circuit(circuit) {
diff --git a/tooling/nargo/src/ops/transform.rs b/tooling/nargo/src/ops/transform.rs
@@ -6,6 +6,7 @@ use iter_extended::vecmap;
 use noirc_driver::{CompiledContract, CompiledProgram};
 use noirc_errors::debug_info::DebugInfo;
 
+/// Apply ACVM optimizations on the circuit.
 pub fn transform_program(
     mut compiled_program: CompiledProgram,
     expression_width: ExpressionWidth,
@@ -18,14 +19,14 @@ pub fn transform_program(
     compiled_program
 }
 
+/// Apply the optimizing transformation on each function in the contract.
 pub fn transform_contract(
     contract: CompiledContract,
     expression_width: ExpressionWidth,
 ) -> CompiledContract {
     let functions = vecmap(contract.functions, |mut func| {
         func.bytecode =
             transform_program_internal(func.bytecode, &mut func.debug, expression_width);
-
         func
     });
 
diff --git a/tooling/nargo_cli/Cargo.toml b/tooling/nargo_cli/Cargo.toml
@@ -25,6 +25,7 @@ toml.workspace = true
 [dependencies]
 clap.workspace = true
 fm.workspace = true
+fxhash.workspace = true
 iter-extended.workspace = true
 nargo.workspace = true
 nargo_fmt.workspace = true
diff --git a/tooling/nargo_cli/src/cli/compile_cmd.rs b/tooling/nargo_cli/src/cli/compile_cmd.rs
diff --git a/tooling/nargo_cli/tests/stdlib-props.rs b/tooling/nargo_cli/tests/stdlib-props.rs
diff --git a/tooling/noirc_abi/src/lib.rs b/tooling/noirc_abi/src/lib.rs
