Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Dec 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 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 @@
}

#[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 Expand Up @@ -165,7 +165,7 @@
match databus {
BlockType::Memory => write!(f, "INIT ")?,
BlockType::CallData(id) => write!(f, "INIT CALLDATA {} ", id)?,
BlockType::ReturnData => write!(f, "INIT RETURNDATA ")?,

Check warning on line 168 in acvm-repo/acir/src/circuit/opcodes.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (RETURNDATA)
}
write!(f, "(id: {}, len: {}) ", block_id.0, init.len())
}
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
25 changes: 20 additions & 5 deletions compiler/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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}");
}
Expand All @@ -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 {
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
2 changes: 1 addition & 1 deletion compiler/noirc_evaluator/src/ssa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
2 changes: 1 addition & 1 deletion tooling/nargo/src/ops/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
3 changes: 2 additions & 1 deletion tooling/nargo/src/ops/transform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
});

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
Loading
Loading