Skip to content

Commit

Permalink
Merge branch 'master' into ab/inline-const-brillig-calls-2
Browse files Browse the repository at this point in the history
  • Loading branch information
asterite authored Nov 21, 2024
2 parents 49135a1 + efa5cc4 commit 43343c9
Show file tree
Hide file tree
Showing 29 changed files with 1,105 additions and 115 deletions.
2 changes: 1 addition & 1 deletion .aztec-sync-commit
Original file line number Diff line number Diff line change
@@ -1 +1 @@
58761fcf181d0a26c4e387105b1bc8a3a75b0368
b8bace9a00c3a8eb93f42682e8cbfa351fc5238c
3 changes: 1 addition & 2 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,9 @@ jobs:
steps:
- name: Run release-please
id: release
uses: google-github-actions/release-please-action@v3
uses: google-github-actions/release-please-action@v4
with:
token: ${{ secrets.NOIR_RELEASES_TOKEN }}
command: manifest

update-acvm-workspace-package-versions:
name: Update acvm workspace package versions
Expand Down
3 changes: 1 addition & 2 deletions .release-please-manifest.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
{
".": "0.39.0",
"acvm-repo": "0.55.0"
".": "0.39.0"
}
4 changes: 2 additions & 2 deletions Cargo.lock

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

47 changes: 24 additions & 23 deletions acvm-repo/acvm/src/compiler/optimizers/merge_expressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,31 +50,36 @@ impl MergeExpressionsOptimizer {
let mut new_circuit = Vec::new();
let mut new_acir_opcode_positions = Vec::new();
// For each opcode, try to get a target opcode to merge with
for (i, opcode) in circuit.opcodes.iter().enumerate() {
for (i, (opcode, opcode_position)) in
circuit.opcodes.iter().zip(acir_opcode_positions).enumerate()
{
if !matches!(opcode, Opcode::AssertZero(_)) {
new_circuit.push(opcode.clone());
new_acir_opcode_positions.push(acir_opcode_positions[i]);
new_acir_opcode_positions.push(opcode_position);
continue;
}
let opcode = modified_gates.get(&i).unwrap_or(opcode).clone();
let mut to_keep = true;
let input_witnesses = self.witness_inputs(&opcode);
for w in input_witnesses.clone() {
let empty_gates = BTreeSet::new();
let gates_using_w = used_witness.get(&w).unwrap_or(&empty_gates);
for w in input_witnesses {
let Some(gates_using_w) = used_witness.get(&w) else {
continue;
};
// We only consider witness which are used in exactly two arithmetic gates
if gates_using_w.len() == 2 {
let gates_using_w: Vec<_> = gates_using_w.iter().collect();
let mut b = *gates_using_w[1];
if b == i {
b = *gates_using_w[0];
let first = *gates_using_w.first().expect("gates_using_w.len == 2");
let second = *gates_using_w.last().expect("gates_using_w.len == 2");
let b = if second == i {
first
} else {
// sanity check
assert!(i == *gates_using_w[0]);
}
let second_gate = modified_gates.get(&b).unwrap_or(&circuit.opcodes[b]).clone();
assert!(i == first);
second
};

let second_gate = modified_gates.get(&b).unwrap_or(&circuit.opcodes[b]);
if let (Opcode::AssertZero(expr_define), Opcode::AssertZero(expr_use)) =
(opcode.clone(), second_gate)
(&opcode, second_gate)
{
// We cannot merge an expression into an earlier opcode, because this
// would break the 'execution ordering' of the opcodes
Expand All @@ -85,16 +90,15 @@ impl MergeExpressionsOptimizer {
// - doing this pass again until there is no change, or
// - merging 'b' into 'i' instead
if i < b {
if let Some(expr) = Self::merge(&expr_use, &expr_define, w) {
if let Some(expr) = Self::merge(expr_use, expr_define, w) {
modified_gates.insert(b, Opcode::AssertZero(expr));
to_keep = false;
// Update the 'used_witness' map to account for the merge.
for w2 in CircuitSimulator::expr_wit(&expr_define) {
for w2 in CircuitSimulator::expr_wit(expr_define) {
if !circuit_inputs.contains(&w2) {
let mut v = used_witness[&w2].clone();
let v = used_witness.entry(w2).or_default();
v.insert(b);
v.remove(&i);
used_witness.insert(w2, v);
}
}
// We need to stop here and continue with the next opcode
Expand All @@ -107,12 +111,9 @@ impl MergeExpressionsOptimizer {
}

if to_keep {
if modified_gates.contains_key(&i) {
new_circuit.push(modified_gates[&i].clone());
} else {
new_circuit.push(opcode.clone());
}
new_acir_opcode_positions.push(acir_opcode_positions[i]);
let opcode = modified_gates.get(&i).cloned().unwrap_or(opcode);
new_circuit.push(opcode);
new_acir_opcode_positions.push(opcode_position);
}
}
(new_circuit, new_acir_opcode_positions)
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ pub fn add_dep(
///
/// This returns a (possibly empty) vector of any warnings found on success.
/// On error, this returns a non-empty vector of warnings and error messages, with at least one error.
#[tracing::instrument(level = "trace", skip(context))]
#[tracing::instrument(level = "trace", skip_all)]
pub fn check_crate(
context: &mut Context,
crate_id: CrateId,
Expand Down
34 changes: 33 additions & 1 deletion compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -806,7 +806,8 @@ fn simplify_derive_generators(
results.push(is_infinite);
}
let len = results.len();
let typ = Type::Array(vec![Type::field()].into(), len);
let typ =
Type::Array(vec![Type::field(), Type::field(), Type::unsigned(1)].into(), len / 3);
let result = make_array(dfg, results.into(), typ, block, call_stack);
SimplifyResult::SimplifiedTo(result)
} else {
Expand All @@ -816,3 +817,34 @@ fn simplify_derive_generators(
unreachable!("Unexpected number of arguments to derive_generators");
}
}

#[cfg(test)]
mod tests {
use crate::ssa::{opt::assert_normalized_ssa_equals, Ssa};

#[test]
fn simplify_derive_generators_has_correct_type() {
let src = "
brillig(inline) fn main f0 {
b0():
v0 = make_array [u8 68, u8 69, u8 70, u8 65, u8 85, u8 76, u8 84, u8 95, u8 68, u8 79, u8 77, u8 65, u8 73, u8 78, u8 95, u8 83, u8 69, u8 80, u8 65, u8 82, u8 65, u8 84, u8 79, u8 82] : [u8; 24]
// This call was previously incorrectly simplified to something that returned `[Field; 3]`
v2 = call derive_pedersen_generators(v0, u32 0) -> [(Field, Field, u1); 1]
return v2
}
";
let ssa = Ssa::from_str(src).unwrap();

let expected = "
brillig(inline) fn main f0 {
b0():
v15 = make_array [u8 68, u8 69, u8 70, u8 65, u8 85, u8 76, u8 84, u8 95, u8 68, u8 79, u8 77, u8 65, u8 73, u8 78, u8 95, u8 83, u8 69, u8 80, u8 65, u8 82, u8 65, u8 84, u8 79, u8 82] : [u8; 24]
v19 = make_array [Field 3728882899078719075161482178784387565366481897740339799480980287259621149274, Field -9903063709032878667290627648209915537972247634463802596148419711785767431332, u1 0] : [(Field, Field, u1); 1]
return v19
}
";
assert_normalized_ssa_equals(ssa, expected);
}
}
5 changes: 5 additions & 0 deletions compiler/noirc_evaluator/src/ssa/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ mod tests;
mod token;

impl Ssa {
/// Creates an Ssa object from the given string.
///
/// Note that the resulting Ssa might not be exactly the same as the given string.
/// This is because, internally, the Ssa is built using a `FunctionBuilder`, so
/// some instructions might be simplified while they are inserted.
pub(crate) fn from_str(src: &str) -> Result<Ssa, SsaErrorWithSource> {
let mut parser =
Parser::new(src).map_err(|err| SsaErrorWithSource::parse_error(err, src))?;
Expand Down
16 changes: 16 additions & 0 deletions compiler/noirc_frontend/src/ast/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ pub struct NoirTrait {
pub items: Vec<Documented<TraitItem>>,
pub attributes: Vec<SecondaryAttribute>,
pub visibility: ItemVisibility,
pub is_alias: bool,
}

/// Any declaration inside the body of a trait that a user is required to
Expand Down Expand Up @@ -77,6 +78,9 @@ pub struct NoirTraitImpl {
pub where_clause: Vec<UnresolvedTraitConstraint>,

pub items: Vec<Documented<TraitImplItem>>,

/// true if generated at compile-time, e.g. from a trait alias
pub is_synthetic: bool,
}

/// Represents a simple trait constraint such as `where Foo: TraitY<U, V>`
Expand Down Expand Up @@ -130,12 +134,19 @@ impl Display for TypeImpl {
}
}

// TODO: display where clauses (follow-up issue)
impl Display for NoirTrait {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let generics = vecmap(&self.generics, |generic| generic.to_string());
let generics = if generics.is_empty() { "".into() } else { generics.join(", ") };

write!(f, "trait {}{}", self.name, generics)?;

if self.is_alias {
let bounds = vecmap(&self.bounds, |bound| bound.to_string()).join(" + ");
return write!(f, " = {};", bounds);
}

if !self.bounds.is_empty() {
let bounds = vecmap(&self.bounds, |bound| bound.to_string()).join(" + ");
write!(f, ": {}", bounds)?;
Expand Down Expand Up @@ -222,6 +233,11 @@ impl Display for TraitBound {

impl Display for NoirTraitImpl {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
// Synthetic NoirTraitImpl's don't get printed
if self.is_synthetic {
return Ok(());
}

write!(f, "impl")?;
if !self.impl_generics.is_empty() {
write!(
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/hir/type_check/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ pub enum TypeCheckError {

#[derive(Debug, Clone, PartialEq, Eq)]
pub struct NoMatchingImplFoundError {
constraints: Vec<(Type, String)>,
pub(crate) constraints: Vec<(Type, String)>,
pub span: Span,
}

Expand Down
2 changes: 2 additions & 0 deletions compiler/noirc_frontend/src/parser/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ pub enum ParserErrorReason {
AssociatedTypesNotAllowedInPaths,
#[error("Associated types are not allowed on a method call")]
AssociatedTypesNotAllowedInMethodCalls,
#[error("Empty trait alias")]
EmptyTraitAlias,
#[error(
"Wrong number of arguments for attribute `{}`. Expected {}, found {}",
name,
Expand Down
2 changes: 2 additions & 0 deletions compiler/noirc_frontend/src/parser/parser/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ impl<'a> Parser<'a> {
let object_type = self.parse_type_or_error();
let where_clause = self.parse_where_clause();
let items = self.parse_trait_impl_body();
let is_synthetic = false;

NoirTraitImpl {
impl_generics,
Expand All @@ -121,6 +122,7 @@ impl<'a> Parser<'a> {
object_type,
where_clause,
items,
is_synthetic,
}
}

Expand Down
Loading

0 comments on commit 43343c9

Please sign in to comment.