Skip to content

Commit

Permalink
fix: detect cycles in globals (noir-lang/noir#6859)
Browse files Browse the repository at this point in the history
chore(ci): Execution time report (noir-lang/noir#6827)
chore(ci): Add non determinism check and fixes (noir-lang/noir#6847)
chore(docs): updating the solidity contract how-to guide (noir-lang/noir#6804)
fix: double alias in path (noir-lang/noir#6855)
feat: configurable external check failures (noir-lang/noir#6810)
chore: move constant creation out of loop (noir-lang/noir#6836)
fix: implement `as_field` and `from_field` in the interpreter (noir-lang/noir#6829)
chore: Use Vec for callstacks (noir-lang/noir#6821)
feat: replace `eval_global_as_array_length` with type/interpreter evaluation (noir-lang/noir#6469)
chore: refactor `DataFlowGraph.insert_instruction_and_results` (noir-lang/noir#6823)
chore(docs): updating noirjs tutorial for 1.0.0 (noir-lang/noir#6792)
feat: Sync from aztec-packages (noir-lang/noir#6824)
chore: Have rust-analyzer use the stable toolchain (noir-lang/noir#6825)
  • Loading branch information
AztecBot committed Dec 19, 2024
2 parents db69894 + 65edaba commit 18d0c7f
Show file tree
Hide file tree
Showing 27 changed files with 454 additions and 121 deletions.
2 changes: 1 addition & 1 deletion .noir-sync-commit
Original file line number Diff line number Diff line change
@@ -1 +1 @@
73ccd45590222fc82642a6a9aa657c2915fc2c58
0d7642cb2071fbee148978a89a0922bfffe5be6a
112 changes: 101 additions & 11 deletions noir/noir-repo/.github/workflows/reports.yml
Original file line number Diff line number Diff line change
Expand Up @@ -225,8 +225,8 @@ jobs:
retention-days: 3
overwrite: true

generate_compilation_report:
name: Compilation time
generate_compilation_and_execution_report:
name: Compilation and execution time
needs: [build-nargo]
runs-on: ubuntu-22.04
permissions:
Expand All @@ -253,34 +253,47 @@ jobs:
working-directory: ./test_programs
run: |
./compilation_report.sh
cat compilation_report.json
mv compilation_report.json ../compilation_report.json
- name: Generate Execution report
working-directory: ./test_programs
run: |
./execution_report.sh
mv execution_report.json ../execution_report.json
- name: Upload compilation report
uses: actions/upload-artifact@v4
with:
name: in_progress_compilation_report
path: compilation_report.json
retention-days: 3
overwrite: true

- name: Upload execution report
uses: actions/upload-artifact@v4
with:
name: in_progress_execution_report
path: execution_report.json
retention-days: 3
overwrite: true

external_repo_compilation_report:
external_repo_compilation_and_execution_report:
needs: [build-nargo]
runs-on: ubuntu-latest
timeout-minutes: 15
strategy:
fail-fast: false
matrix:
include:
- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-contracts }
- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-contracts, is_library: true }
- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/parity-root }
- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/private-kernel-inner }
- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/private-kernel-tail }
- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/private-kernel-reset }
- 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 }

name: External repo compilation report - ${{ matrix.project.repo }}/${{ matrix.project.path }}
name: External repo compilation and execution reports - ${{ matrix.project.repo }}/${{ matrix.project.path }}
steps:
- name: Download nargo binary
uses: actions/download-artifact@v4
Expand All @@ -302,6 +315,13 @@ jobs:
sparse-checkout: |
test_programs/compilation_report.sh
sparse-checkout-cone-mode: false

- uses: actions/checkout@v4
with:
path: scripts
sparse-checkout: |
test_programs/execution_report.sh
sparse-checkout-cone-mode: false

- name: Checkout
uses: actions/checkout@v4
Expand All @@ -316,28 +336,53 @@ jobs:
mv /home/runner/work/noir/noir/scripts/test_programs/compilation_report.sh ./compilation_report.sh
chmod +x ./compilation_report.sh
./compilation_report.sh 1
- name: Generate execution report
working-directory: ./test-repo/${{ matrix.project.path }}
if: ${{ !matrix.project.is_library }}
run: |
mv /home/runner/work/noir/noir/scripts/test_programs/execution_report.sh ./execution_report.sh
./execution_report.sh 1
- name: Move compilation report
id: report
id: compilation_report
shell: bash
run: |
PACKAGE_NAME=${{ matrix.project.path }}
PACKAGE_NAME=$(basename $PACKAGE_NAME)
mv ./test-repo/${{ matrix.project.path }}/compilation_report.json ./compilation_report_$PACKAGE_NAME.json
echo "compilation_report_name=$PACKAGE_NAME" >> $GITHUB_OUTPUT
- name: Move execution report
id: execution_report
shell: bash
if: ${{ !matrix.project.is_library }}
run: |
PACKAGE_NAME=${{ matrix.project.path }}
PACKAGE_NAME=$(basename $PACKAGE_NAME)
mv ./test-repo/${{ matrix.project.path }}/execution_report.json ./execution_report_$PACKAGE_NAME.json
echo "execution_report_name=$PACKAGE_NAME" >> $GITHUB_OUTPUT
- name: Upload compilation report
uses: actions/upload-artifact@v4
with:
name: compilation_report_${{ steps.report.outputs.compilation_report_name }}
path: compilation_report_${{ steps.report.outputs.compilation_report_name }}.json
name: compilation_report_${{ steps.compilation_report.outputs.compilation_report_name }}
path: compilation_report_${{ steps.compilation_report.outputs.compilation_report_name }}.json
retention-days: 3
overwrite: true

- name: Upload execution report
uses: actions/upload-artifact@v4
with:
name: execution_report_${{ steps.execution_report.outputs.execution_report_name }}
path: execution_report_${{ steps.execution_report.outputs.execution_report_name }}.json
retention-days: 3
overwrite: true

upload_compilation_report:
name: Upload compilation report
needs: [generate_compilation_report, external_repo_compilation_report]
# We want this job to run even if one variation of the matrix in `external_repo_compilation_report` fails
needs: [generate_compilation_and_execution_report, external_repo_compilation_and_execution_report]
# We want this job to run even if one variation of the matrix in `external_repo_compilation_and_execution_report` fails
if: always()
runs-on: ubuntu-latest
permissions:
Expand Down Expand Up @@ -490,3 +535,48 @@ jobs:
with:
header: memory
message: ${{ steps.memory_report.outputs.markdown }}

upload_execution_report:
name: Upload execution report
needs: [generate_compilation_and_execution_report, external_repo_compilation_and_execution_report]
# We want this job to run even if one variation of the matrix in `external_repo_compilation_and_execution_report` fails
if: always()
runs-on: ubuntu-latest
permissions:
pull-requests: write

steps:
- uses: actions/checkout@v4

- name: Download initial execution report
uses: actions/download-artifact@v4
with:
name: in_progress_execution_report

- name: Download matrix execution reports
uses: actions/download-artifact@v4
with:
pattern: execution_report_*
path: ./reports

- name: Merge execution reports using jq
run: |
mv ./.github/scripts/merge-bench-reports.sh merge-bench-reports.sh
./merge-bench-reports.sh execution_report
- name: Parse execution report
id: execution_report
uses: noir-lang/noir-bench-report@0954121203ee55dcda5c7397b9c669c439a20922
with:
report: execution_report.json
header: |
# Execution Report
execution_report: true

- name: Add memory report to sticky comment
if: github.event_name == 'pull_request' || github.event_name == 'pull_request_target'
uses: marocchino/sticky-pull-request-comment@v2
with:
header: execution_time
message: ${{ steps.execution_report.outputs.markdown }}

1 change: 1 addition & 0 deletions noir/noir-repo/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ gates_report.json
gates_report_brillig.json
gates_report_brillig_execution.json
compilation_report.json
execution_report.json

# Github Actions scratch space
# This gives a location to download artifacts into the repository in CI without making git dirty.
Expand Down
4 changes: 4 additions & 0 deletions noir/noir-repo/compiler/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,10 @@ pub struct CompileOptions {
/// A lower value keeps the original program if it was smaller, even if it has more jumps.
#[arg(long, hide = true, allow_hyphen_values = true)]
pub max_bytecode_increase_percent: Option<i32>,

/// Used internally to test for non-determinism in the compiler.
#[clap(long, hide = true)]
pub check_non_determinism: bool,
}

pub fn parse_expression_width(input: &str) -> Result<ExpressionWidth, std::io::Error> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ pub(crate) struct GeneratedBrillig<F> {
pub(crate) locations: BTreeMap<OpcodeLocation, CallStack>,
pub(crate) error_types: BTreeMap<ErrorSelector, ErrorType>,
pub(crate) name: String,
pub(crate) procedure_locations: HashMap<ProcedureId, (OpcodeLocation, OpcodeLocation)>,
pub(crate) procedure_locations: BTreeMap<ProcedureId, (OpcodeLocation, OpcodeLocation)>,
}

#[derive(Default, Debug, Clone)]
Expand Down Expand Up @@ -61,7 +61,7 @@ pub(crate) struct BrilligArtifact<F> {
/// This is created as artifacts are linked together and allows us to determine
/// which opcodes originate from reusable procedures.s
/// The range is inclusive for both start and end opcode locations.
pub(crate) procedure_locations: HashMap<ProcedureId, (OpcodeLocation, OpcodeLocation)>,
pub(crate) procedure_locations: BTreeMap<ProcedureId, (OpcodeLocation, OpcodeLocation)>,
}

/// A pointer to a location in the opcode.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
//!
//! When unrolling ACIR code, we remove reference count instructions because they are
//! only used by Brillig bytecode.
use std::collections::BTreeSet;

use acvm::{acir::AcirField, FieldElement};
use im::HashSet;

Expand Down Expand Up @@ -117,7 +119,7 @@ pub(super) struct Loop {
back_edge_start: BasicBlockId,

/// All the blocks contained within the loop, including `header` and `back_edge_start`.
pub(super) blocks: HashSet<BasicBlockId>,
pub(super) blocks: BTreeSet<BasicBlockId>,
}

pub(super) struct Loops {
Expand Down Expand Up @@ -238,7 +240,7 @@ impl Loop {
back_edge_start: BasicBlockId,
cfg: &ControlFlowGraph,
) -> Self {
let mut blocks = HashSet::default();
let mut blocks = BTreeSet::default();
blocks.insert(header);

let mut insert = |block, stack: &mut Vec<BasicBlockId>| {
Expand Down
13 changes: 6 additions & 7 deletions noir/noir-repo/compiler/noirc_frontend/src/ast/statement.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use std::fmt::Display;
use std::sync::atomic::{AtomicU32, Ordering};

use acvm::acir::AcirField;
use acvm::FieldElement;
Expand Down Expand Up @@ -841,10 +840,9 @@ impl ForRange {
block: Expression,
for_loop_span: Span,
) -> Statement {
/// Counter used to generate unique names when desugaring
/// code in the parser requires the creation of fresh variables.
/// The parser is stateless so this is a static global instead.
static UNIQUE_NAME_COUNTER: AtomicU32 = AtomicU32::new(0);
// Counter used to generate unique names when desugaring
// code in the parser requires the creation of fresh variables.
let mut unique_name_counter: u32 = 0;

match self {
ForRange::Range(..) => {
Expand All @@ -855,7 +853,8 @@ impl ForRange {
let start_range = ExpressionKind::integer(FieldElement::zero());
let start_range = Expression::new(start_range, array_span);

let next_unique_id = UNIQUE_NAME_COUNTER.fetch_add(1, Ordering::Relaxed);
let next_unique_id = unique_name_counter;
unique_name_counter += 1;
let array_name = format!("$i{next_unique_id}");
let array_span = array.span;
let array_ident = Ident::new(array_name, array_span);
Expand Down Expand Up @@ -888,7 +887,7 @@ impl ForRange {
}));
let end_range = Expression::new(end_range, array_span);

let next_unique_id = UNIQUE_NAME_COUNTER.fetch_add(1, Ordering::Relaxed);
let next_unique_id = unique_name_counter;
let index_name = format!("$i{next_unique_id}");
let fresh_identifier = Ident::new(index_name.clone(), array_span);

Expand Down
16 changes: 13 additions & 3 deletions noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ use std::{
};

use crate::{
ast::ItemVisibility, hir_def::traits::ResolvedTraitBound, usage_tracker::UsageTracker,
StructField, StructType, TypeBindings,
ast::ItemVisibility, hir_def::traits::ResolvedTraitBound, node_interner::GlobalValue,
usage_tracker::UsageTracker, StructField, StructType, TypeBindings,
};
use crate::{
ast::{
Expand Down Expand Up @@ -1689,7 +1689,17 @@ impl<'context> Elaborator<'context> {

self.debug_comptime(location, |interner| value.display(interner).to_string());

self.interner.get_global_mut(global_id).value = Some(value);
self.interner.get_global_mut(global_id).value = GlobalValue::Resolved(value);
}
}

/// If the given global is unresolved, elaborate it and return true
fn elaborate_global_if_unresolved(&mut self, global_id: &GlobalId) -> bool {
if let Some(global) = self.unresolved_globals.remove(global_id) {
self.elaborate_global(global);
true
} else {
false
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::hir::resolution::visibility::item_in_module_is_visible;

use crate::locations::ReferencesTracker;
use crate::node_interner::{FuncId, GlobalId, StructId, TraitId, TypeAliasId};
use crate::Type;
use crate::{Shared, Type, TypeAlias};

use super::types::SELF_TYPE_NAME;
use super::Elaborator;
Expand Down Expand Up @@ -218,18 +218,8 @@ impl<'context> Elaborator<'context> {
),
ModuleDefId::TypeAliasId(id) => {
let type_alias = self.interner.get_type_alias(id);
let type_alias = type_alias.borrow();

let module_id = match &type_alias.typ {
Type::Struct(struct_id, _generics) => struct_id.borrow().id.module_id(),
Type::Error => {
return Err(PathResolutionError::Unresolved(last_ident.clone()));
}
_ => {
// For now we only allow type aliases that point to structs.
// The more general case is captured here: https://github.com/noir-lang/noir/issues/6398
panic!("Type alias in path not pointing to struct not yet supported")
}
let Some(module_id) = get_type_alias_module_def_id(&type_alias) else {
return Err(PathResolutionError::Unresolved(last_ident.clone()));
};

(
Expand Down Expand Up @@ -345,3 +335,18 @@ fn merge_intermediate_path_resolution_item_with_module_def_id(
},
}
}

fn get_type_alias_module_def_id(type_alias: &Shared<TypeAlias>) -> Option<ModuleId> {
let type_alias = type_alias.borrow();

match &type_alias.typ {
Type::Struct(struct_id, _generics) => Some(struct_id.borrow().id.module_id()),
Type::Alias(type_alias, _generics) => get_type_alias_module_def_id(type_alias),
Type::Error => None,
_ => {
// For now we only allow type aliases that point to structs.
// The more general case is captured here: https://github.com/noir-lang/noir/issues/6398
panic!("Type alias in path not pointing to struct not yet supported")
}
}
}
Loading

0 comments on commit 18d0c7f

Please sign in to comment.