Skip to content

Commit

Permalink
Skip post-compilation transformations if we loaded from cache
Browse files Browse the repository at this point in the history
  • Loading branch information
aakoshh committed Dec 3, 2024
1 parent a1cac95 commit 18d6533
Show file tree
Hide file tree
Showing 16 changed files with 77 additions and 46 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions acvm-repo/acir/src/circuit/brillig.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>>),
Expand All @@ -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>),
Expand All @@ -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>>,
}
Expand Down
12 changes: 6 additions & 6 deletions acvm-repo/acir/src/circuit/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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.)
Expand All @@ -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>>,
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions acvm-repo/acir/src/circuit/opcodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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
Expand Down
6 changes: 3 additions & 3 deletions acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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>>,
Expand Down
2 changes: 1 addition & 1 deletion acvm-repo/acir/src/circuit/opcodes/memory_operation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down
2 changes: 1 addition & 1 deletion acvm-repo/brillig/src/black_box.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
18 changes: 9 additions & 9 deletions acvm-repo/brillig/src/opcodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_driver/src/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
22 changes: 14 additions & 8 deletions compiler/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -558,8 +558,11 @@ pub const DEFAULT_EXPRESSION_WIDTH: ExpressionWidth = ExpressionWidth::Bounded {
///
/// This function assumes [`check_crate`] is called beforehand.
///
/// The returned program is backend-agnostic and so must go through a transformation pass before usage in proof generation.
/// These transformations are _not_ covered by the check that decides whether we can use the cached artifact.
/// 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.
Expand All @@ -577,9 +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}");
}
Expand All @@ -593,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 {
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_driver/src/program.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`]
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_errors/src/debug_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions compiler/noirc_evaluator/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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 },
Expand Down
1 change: 1 addition & 0 deletions tooling/nargo_cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
24 changes: 23 additions & 1 deletion tooling/nargo_cli/src/cli/compile_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,18 +189,40 @@ fn compile_programs(
};

let compile_package = |package| {
let cached_program = load_cached_program(package);

// Hash over the entire compiled program, including any post-compile transformations.
// This is used to detect whether `cached_program` is returned by `compile_program`.
let cached_hash = cached_program.as_ref().map(fxhash::hash64);

// Compile the program, or use the cached artifacts if it matches.
let (program, warnings) = compile_program(
file_manager,
parsed_files,
workspace,
package,
compile_options,
load_cached_program(package),
cached_program,
)?;

// Choose the target width for the final, backend specific transformation.
let target_width =
get_target_width(package.expression_width, compile_options.expression_width);

// If the compiled program is the same as the cached one, we don't apply transformations again, unless the target width has changed.
// The transformations might not be idempotent, which would risk creating witnesses that don't work with earlier versions,
// based on which we might have generated a verifier already.
if cached_hash == Some(fxhash::hash64(&program)) {
let width_matches = program
.program
.functions
.iter()
.all(|circuit| circuit.expression_width == target_width);

if width_matches {
return Ok(((), warnings));
}
}
// Run ACVM optimizations and set the target width.
let program = nargo::ops::transform_program(program, target_width);
// Check solvability.
Expand Down
Loading

0 comments on commit 18d6533

Please sign in to comment.