Skip to content

Commit

Permalink
Merge branch 'master' into 6742-optional-mock
Browse files Browse the repository at this point in the history
  • Loading branch information
aakoshh authored Jan 10, 2025
2 parents 5d4cc26 + 24433fe commit 7b53dae
Show file tree
Hide file tree
Showing 33 changed files with 371 additions and 234 deletions.
8 changes: 4 additions & 4 deletions .github/workflows/reports.yml
Original file line number Diff line number Diff line change
Expand Up @@ -303,11 +303,11 @@ jobs:
- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/private-kernel-inner, take_average: true }
- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/private-kernel-tail, take_average: true }
- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/private-kernel-reset, take_average: true }
- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-root }
- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-root, take_average: true }
#- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-block-root }
- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-block-merge }
- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-base-private }
- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-base-public }
- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-block-merge, take_average: true }
- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-base-private, take_average: true }
- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-base-public, take_average: true }

name: External repo compilation and execution reports - ${{ matrix.project.repo }}/${{ matrix.project.path }}
steps:
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_evaluator/src/acir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2169,7 +2169,7 @@ impl<'a> Context<'a> {

let Type::Array(result_type, array_length) = dfg.type_of_value(result_ids[0])
else {
unreachable!("ICE: ToRadix result must be an array");
unreachable!("ICE: ToBits result must be an array");
};

self.acir_context
Expand Down
1 change: 0 additions & 1 deletion compiler/noirc_evaluator/src/ssa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,6 @@ fn optimize_all(builder: SsaBuilder, options: &SsaEvaluatorOptions) -> Result<Ss
.run_pass(Ssa::remove_unreachable_functions, "Removing Unreachable Functions")
.run_pass(Ssa::defunctionalize, "Defunctionalization")
.run_pass(Ssa::remove_paired_rc, "Removing Paired rc_inc & rc_decs")
.run_pass(Ssa::resolve_is_unconstrained, "Resolving IsUnconstrained")
.run_pass(|ssa| ssa.inline_functions(options.inliner_aggressiveness), "Inlining (1st)")
// Run mem2reg with the CFG separated into blocks
.run_pass(Ssa::mem2reg, "Mem2Reg (1st)")
Expand Down
1 change: 1 addition & 0 deletions compiler/noirc_evaluator/src/ssa/ir/dfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,7 @@ impl DataFlowGraph {

/// Remove an instruction by replacing it with a `Noop` instruction.
/// Doing this avoids shifting over each instruction after this one in its block's instructions vector.
#[allow(unused)]
pub(crate) fn remove_instruction(&mut self, instruction: InstructionId) {
self.instructions[instruction] = Instruction::Noop;
self.results.insert(instruction, smallvec::SmallVec::new());
Expand Down
5 changes: 4 additions & 1 deletion compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,10 @@ pub(super) fn simplify_call(
simplify_black_box_func(bb_func, arguments, dfg, block, call_stack)
}
Intrinsic::AsWitness => SimplifyResult::None,
Intrinsic::IsUnconstrained => SimplifyResult::None,
Intrinsic::IsUnconstrained => {
let result = dfg.runtime().is_brillig().into();
SimplifyResult::SimplifiedTo(dfg.make_constant(result, NumericType::bool()))
}
Intrinsic::DerivePedersenGenerators => {
if let Some(Type::Array(_, len)) = return_type.clone() {
simplify_derive_generators(dfg, arguments, len, block, call_stack)
Expand Down
16 changes: 7 additions & 9 deletions compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1557,7 +1557,6 @@ mod test {
// After EnableSideEffectsIf removal:
brillig(inline) fn main f0 {
b0(v0: Field, v1: Field, v2: u1):
v4 = call is_unconstrained() -> u1
v7 = call to_be_radix(v0, u32 256) -> [u8; 1] // `a.to_be_radix(256)`;
inc_rc v7
v8 = call to_be_radix(v0, u32 256) -> [u8; 1] // duplicate load of `a`
Expand All @@ -1574,14 +1573,13 @@ mod test {
let expected = "
brillig(inline) fn main f0 {
b0(v0: Field, v1: Field, v2: u1):
v4 = call is_unconstrained() -> u1
v7 = call to_be_radix(v0, u32 256) -> [u8; 1]
inc_rc v7
inc_rc v7
v8 = cast v2 as Field
v9 = mul v0, v8
v10 = call to_be_radix(v9, u32 256) -> [u8; 1]
inc_rc v10
v5 = call to_be_radix(v0, u32 256) -> [u8; 1]
inc_rc v5
inc_rc v5
v6 = cast v2 as Field
v7 = mul v0, v6
v8 = call to_be_radix(v7, u32 256) -> [u8; 1]
inc_rc v8
enable_side_effects v2
return
}
Expand Down
1 change: 0 additions & 1 deletion compiler/noirc_evaluator/src/ssa/opt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ mod remove_bit_shifts;
mod remove_enable_side_effects;
mod remove_if_else;
mod remove_unreachable;
mod resolve_is_unconstrained;
mod simplify_cfg;
mod unrolling;

Expand Down
55 changes: 0 additions & 55 deletions compiler/noirc_evaluator/src/ssa/opt/resolve_is_unconstrained.rs

This file was deleted.

23 changes: 23 additions & 0 deletions compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ pub(crate) mod context;
mod program;
mod value;

use acvm::AcirField;
use noirc_frontend::token::FmtStrFragment;
pub(crate) use program::Ssa;

Expand Down Expand Up @@ -595,6 +596,9 @@ impl<'a> FunctionContext<'a> {
/// ```
fn codegen_if(&mut self, if_expr: &ast::If) -> Result<Values, RuntimeError> {
let condition = self.codegen_non_tuple_expression(&if_expr.condition)?;
if let Some(result) = self.try_codegen_constant_if(condition, if_expr) {
return result;
}

let then_block = self.builder.insert_block();
let else_block = self.builder.insert_block();
Expand Down Expand Up @@ -633,6 +637,25 @@ impl<'a> FunctionContext<'a> {
Ok(result)
}

/// If the condition is known, skip codegen for the then/else branch and only compile the
/// relevant branch.
fn try_codegen_constant_if(
&mut self,
condition: ValueId,
if_expr: &ast::If,
) -> Option<Result<Values, RuntimeError>> {
let condition = self.builder.current_function.dfg.get_numeric_constant(condition)?;

Some(if condition.is_zero() {
match if_expr.alternative.as_ref() {
Some(alternative) => self.codegen_expression(alternative),
None => Ok(Self::unit_value()),
}
} else {
self.codegen_expression(&if_expr.consequence)
})
}

fn codegen_tuple(&mut self, tuple: &[Expression]) -> Result<Values, RuntimeError> {
Ok(Tree::Branch(try_vecmap(tuple, |expr| self.codegen_expression(expr))?))
}
Expand Down
83 changes: 42 additions & 41 deletions compiler/noirc_frontend/src/elaborator/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@ use crate::{
TraitImplKind, TraitMethodId,
},
token::SecondaryAttribute,
Generics, Kind, ResolvedGeneric, Shared, StructType, Type, TypeBinding, TypeBindings,
UnificationError,
Generics, Kind, ResolvedGeneric, Type, TypeBinding, TypeBindings, UnificationError,
};

use super::{lints, path_resolution::PathResolutionItem, Elaborator, UnsafeBlockStatus};
Expand Down Expand Up @@ -1320,9 +1319,6 @@ impl<'context> Elaborator<'context> {
has_self_arg: bool,
) -> Option<HirMethodReference> {
match object_type.follow_bindings() {
Type::Struct(typ, _args) => {
self.lookup_struct_method(object_type, method_name, span, has_self_arg, typ)
}
// TODO: We should allow method calls on `impl Trait`s eventually.
// For now it is fine since they are only allowed on return types.
Type::TraitAsType(..) => {
Expand All @@ -1338,11 +1334,9 @@ impl<'context> Elaborator<'context> {
}
// Mutable references to another type should resolve to methods of their element type.
// This may be a struct or a primitive type.
Type::MutableReference(element) => self
.interner
.lookup_primitive_trait_method_mut(element.as_ref(), method_name, has_self_arg)
.map(HirMethodReference::FuncId)
.or_else(|| self.lookup_method(&element, method_name, span, has_self_arg)),
Type::MutableReference(element) => {
self.lookup_method(&element, method_name, span, has_self_arg)
}

// If we fail to resolve the object to a struct type, we have no way of type
// checking its arguments as we can't even resolve the name of the function
Expand All @@ -1354,51 +1348,45 @@ impl<'context> Elaborator<'context> {
None
}

other => match self.interner.lookup_primitive_method(&other, method_name, has_self_arg)
{
Some(method_id) => Some(HirMethodReference::FuncId(method_id)),
None => {
// It could be that this type is a composite type that is bound to a trait,
// for example `x: (T, U) ... where (T, U): SomeTrait`
// (so this case is a generalization of the NamedGeneric case)
self.lookup_method_in_trait_constraints(object_type, method_name, span)
}
},
other => {
self.lookup_struct_or_primitive_method(&other, method_name, span, has_self_arg)
}
}
}

fn lookup_struct_method(
fn lookup_struct_or_primitive_method(
&mut self,
object_type: &Type,
method_name: &str,
span: Span,
has_self_arg: bool,
typ: Shared<StructType>,
) -> Option<HirMethodReference> {
let id = typ.borrow().id;

// First search in the struct methods
// First search in the type methods. If there is one, that's the one.
if let Some(method_id) =
self.interner.lookup_direct_method(object_type, id, method_name, has_self_arg)
self.interner.lookup_direct_method(object_type, method_name, has_self_arg)
{
return Some(HirMethodReference::FuncId(method_id));
}

// Next lookup all matching trait methods.
let trait_methods =
self.interner.lookup_trait_methods(object_type, id, method_name, has_self_arg);
self.interner.lookup_trait_methods(object_type, method_name, has_self_arg);

if trait_methods.is_empty() {
// If we couldn't find any trait methods, search in
// impls for all types `T`, e.g. `impl<T> Foo for T`
if let Some(func_id) =
self.interner.lookup_generic_method(object_type, method_name, has_self_arg)
{
return Some(HirMethodReference::FuncId(func_id));
}
// If there's at least one matching trait method we need to see if only one is in scope.
if !trait_methods.is_empty() {
return self.return_trait_method_in_scope(&trait_methods, method_name, span);
}

// If we couldn't find any trait methods, search in
// impls for all types `T`, e.g. `impl<T> Foo for T`
let generic_methods =
self.interner.lookup_generic_methods(object_type, method_name, has_self_arg);
if !generic_methods.is_empty() {
return self.return_trait_method_in_scope(&generic_methods, method_name, span);
}

// Otherwise it's an error
let has_field_with_function_type = typ
if let Type::Struct(struct_type, _) = object_type {
let has_field_with_function_type = struct_type
.borrow()
.get_fields_as_written()
.into_iter()
Expand All @@ -1416,10 +1404,23 @@ impl<'context> Elaborator<'context> {
span,
});
}
return None;
None
} else {
// It could be that this type is a composite type that is bound to a trait,
// for example `x: (T, U) ... where (T, U): SomeTrait`
// (so this case is a generalization of the NamedGeneric case)
self.lookup_method_in_trait_constraints(object_type, method_name, span)
}
}

// We found some trait methods... but is only one of them currently in scope?
/// Given a list of functions and the trait they belong to, returns the one function
/// that is in scope.
fn return_trait_method_in_scope(
&mut self,
trait_methods: &[(FuncId, TraitId)],
method_name: &str,
span: Span,
) -> Option<HirMethodReference> {
let module_id = self.module_id();
let module_data = self.get_module(module_id);

Expand Down Expand Up @@ -1501,7 +1502,7 @@ impl<'context> Elaborator<'context> {
fn trait_hir_method_reference(
&self,
trait_id: TraitId,
trait_methods: Vec<(FuncId, TraitId)>,
trait_methods: &[(FuncId, TraitId)],
method_name: &str,
span: Span,
) -> HirMethodReference {
Expand All @@ -1526,7 +1527,7 @@ impl<'context> Elaborator<'context> {
) -> Option<HirMethodReference> {
let func_id = match self.current_item {
Some(DependencyId::Function(id)) => id,
_ => panic!("unexpected method outside a function"),
_ => panic!("unexpected method outside a function: {method_name}"),
};
let func_meta = self.interner.function_meta(&func_id);

Expand Down
22 changes: 22 additions & 0 deletions compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,8 @@ impl<'local, 'context> Interpreter<'local, 'context> {
"struct_def_set_fields" => struct_def_set_fields(interner, arguments, location),
"to_be_radix" => to_be_radix(arguments, return_type, location),
"to_le_radix" => to_le_radix(arguments, return_type, location),
"to_be_bits" => to_be_bits(arguments, return_type, location),
"to_le_bits" => to_le_bits(arguments, return_type, location),
"trait_constraint_eq" => trait_constraint_eq(arguments, location),
"trait_constraint_hash" => trait_constraint_hash(arguments, location),
"trait_def_as_trait_constraint" => {
Expand Down Expand Up @@ -776,6 +778,26 @@ fn quoted_tokens(arguments: Vec<(Value, Location)>, location: Location) -> IResu
))
}

fn to_be_bits(
arguments: Vec<(Value, Location)>,
return_type: Type,
location: Location,
) -> IResult<Value> {
let value = check_one_argument(arguments, location)?;
let radix = (Value::U32(2), value.1);
to_be_radix(vec![value, radix], return_type, location)
}

fn to_le_bits(
arguments: Vec<(Value, Location)>,
return_type: Type,
location: Location,
) -> IResult<Value> {
let value = check_one_argument(arguments, location)?;
let radix = (Value::U32(2), value.1);
to_le_radix(vec![value, radix], return_type, location)
}

fn to_be_radix(
arguments: Vec<(Value, Location)>,
return_type: Type,
Expand Down
2 changes: 2 additions & 0 deletions compiler/noirc_frontend/src/hir/comptime/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ use crate::node_interner::FuncId;
use crate::parse_program;

/// Create an interpreter for a code snippet and pass it to a test function.
///
/// The stdlib is not made available as a dependency.
pub(crate) fn with_interpreter<T>(
src: &str,
f: impl FnOnce(&mut Interpreter, FuncId, &[(CompilationError, FileId)]) -> T,
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/hir_def/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1862,7 +1862,7 @@ impl Type {
if let (Type::Array(_size, element1), Type::Slice(element2)) = (&this, &target) {
// We can only do the coercion if the `as_slice` method exists.
// This is usually true, but some tests don't have access to the standard library.
if let Some(as_slice) = interner.lookup_primitive_method(&this, "as_slice", true) {
if let Some(as_slice) = interner.lookup_direct_method(&this, "as_slice", true) {
// Still have to ensure the element types match.
// Don't need to issue an error here if not, it will be done in unify_with_coercions
let mut bindings = TypeBindings::new();
Expand Down
Loading

0 comments on commit 7b53dae

Please sign in to comment.