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: Sync from noir #11138

Merged
merged 28 commits into from
Jan 11, 2025
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
b590871
[1 changes] feat!: require trait primitive functions/calls to have th…
AztecBot Jan 9, 2025
a8a7cba
chore: apply sync fixes
AztecBot Jan 9, 2025
e791a61
Merge branch 'master' into sync-noir
TomAFrench Jan 9, 2025
a240753
.
TomAFrench Jan 9, 2025
5e46b37
.
TomAFrench Jan 9, 2025
e02ffac
Merge branch 'master' into sync-noir
TomAFrench Jan 9, 2025
a95a252
.
TomAFrench Jan 9, 2025
479c38b
[1 changes] fix: require generic trait impls to be in scope to call t…
AztecBot Jan 10, 2025
e9c5985
chore: apply sync fixes
AztecBot Jan 10, 2025
271966d
fix: require generic trait impls to be in scope to call them (https:/…
AztecBot Jan 10, 2025
f93468f
Merge branch 'master' into sync-noir
TomAFrench Jan 10, 2025
273e594
Merge branch 'master' into sync-noir
TomAFrench Jan 10, 2025
cd0972b
[1 changes] feat: unchecked math operations in SSA (https://github.co…
AztecBot Jan 10, 2025
68ba915
chore: apply sync fixes
AztecBot Jan 10, 2025
3313783
feat: unchecked math operations in SSA (https://github.com/noir-lang/…
AztecBot Jan 10, 2025
7db0b11
.
TomAFrench Jan 10, 2025
d7f8bb3
.
TomAFrench Jan 10, 2025
d876b5f
Merge branch 'master' into sync-noir
TomAFrench Jan 10, 2025
2b2f306
Merge branch 'master' into sync-noir
TomAFrench Jan 10, 2025
948aa4d
Merge branch 'master' into sync-noir
TomAFrench Jan 10, 2025
65969cb
Merge branch 'master' into sync-noir
TomAFrench Jan 10, 2025
0f02951
[1 changes] feat: SSA globals in monomorphization and SSA gen (https…
AztecBot Jan 11, 2025
0ea85f5
chore: apply sync fixes
AztecBot Jan 11, 2025
d10c025
feat: SSA globals in monomorphization and SSA gen (https://github.co…
AztecBot Jan 11, 2025
7be7e78
Merge branch 'master' into sync-noir
TomAFrench Jan 11, 2025
19bb8f5
.
TomAFrench Jan 11, 2025
008d474
.
TomAFrench Jan 11, 2025
f435c39
.
TomAFrench Jan 11, 2025
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
2 changes: 1 addition & 1 deletion .noir-sync-commit
Original file line number Diff line number Diff line change
@@ -1 +1 @@
3c488f4b272f460383341c51270b87bfe2b94468
56c931a8e59e9e67e8bd5aae4a114e85fe40ff98
3 changes: 2 additions & 1 deletion noir/noir-repo/.github/scripts/check_test_results.sh
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ if [ -f $1 ] && [ -f $2 ]; then
echo "Error: test failures don't match expected failures"
echo "Lines prefixed with '>' are new test failures (you could add them to '$1')"
echo "Lines prefixed with '<' are tests that were expected to fail but passed (you could remove them from '$1')"
exit -1
fi
elif [ -f $1 ]; then
# Only the expected file exists, which means the actual test couldn't be compiled.
Expand All @@ -35,4 +36,4 @@ else
# Both files don't exists, which means we are expecting the external library not
# to compile, and it didn't, so all is good.
exit 0
fi
fi
38 changes: 19 additions & 19 deletions noir/noir-repo/.github/workflows/reports.yml
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ jobs:
./rebuild.sh
./gates_report.sh
mv gates_report.json ../gates_report.json

- name: Compare gates reports
id: gates_diff
uses: noir-lang/noir-gates-diff@7e4ddaa91c69380f15ccba514eac17bc7432a8cc
Expand Down Expand Up @@ -139,7 +139,7 @@ jobs:
message: ${{ steps.brillig_bytecode_diff.outputs.markdown }}

compare_brillig_execution_reports:
name: Brillig execution trace sizes
name: Brillig execution trace sizes
needs: [build-nargo]
runs-on: ubuntu-22.04
permissions:
Expand Down Expand Up @@ -188,7 +188,7 @@ jobs:
message: ${{ steps.brillig_execution_diff.outputs.markdown }}

generate_memory_report:
name: Peak memory usage
name: Peak memory usage
needs: [build-nargo]
runs-on: ubuntu-22.04
permissions:
Expand Down Expand Up @@ -274,15 +274,15 @@ jobs:
run: |
./execution_report.sh 0 1
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:
Expand All @@ -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 Expand Up @@ -360,7 +360,7 @@ jobs:
chmod +x parse_time.sh
cp /home/runner/work/noir/noir/scripts/test_programs/parse_time.sh ./parse_time.sh
./compilation_report.sh 1 1

- name: Generate execution report without averages
working-directory: ./test-repo/${{ matrix.project.path }}
if: ${{ !matrix.project.is_library && !matrix.project.take_average }}
Expand Down Expand Up @@ -395,7 +395,7 @@ jobs:
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:
Expand All @@ -413,10 +413,10 @@ jobs:
overwrite: true

upload_compilation_report:
name: Upload compilation report
name: Upload compilation 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()
if: always()
runs-on: ubuntu-latest
permissions:
pull-requests: write
Expand Down Expand Up @@ -550,10 +550,10 @@ jobs:
overwrite: true

upload_compilation_memory_report:
name: Upload compilation memory report
name: Upload compilation memory report
needs: [generate_memory_report, external_repo_memory_report]
# We want this job to run even if one variation of the matrix in `external_repo_memory_report` fails
if: always()
if: always()
runs-on: ubuntu-latest
permissions:
pull-requests: write
Expand Down Expand Up @@ -594,10 +594,10 @@ jobs:
message: ${{ steps.compilation_mem_report.outputs.markdown }}

upload_execution_memory_report:
name: Upload execution memory report
name: Upload execution memory report
needs: [generate_memory_report, external_repo_memory_report]
# We want this job to run even if one variation of the matrix in `external_repo_memory_report` fails
if: always()
if: always()
runs-on: ubuntu-latest
permissions:
pull-requests: write
Expand Down Expand Up @@ -640,10 +640,10 @@ jobs:
message: ${{ steps.execution_mem_report.outputs.markdown }}

upload_execution_report:
name: 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()
if: always()
runs-on: ubuntu-latest
permissions:
pull-requests: write
Expand Down
19 changes: 12 additions & 7 deletions noir/noir-repo/.github/workflows/test-js-packages.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:
yarn-lock:
runs-on: ubuntu-22.04
timeout-minutes: 30

steps:
- name: Checkout
uses: actions/checkout@v4
Expand Down Expand Up @@ -492,7 +492,7 @@ jobs:
uses: foundry-rs/[email protected]
with:
version: nightly-8660e5b941fe7f4d67e246cfd3dafea330fb53b1


- name: Install `bb`
run: |
Expand Down Expand Up @@ -530,10 +530,10 @@ jobs:
steps:
- name: Checkout
uses: actions/checkout@v4

- name: Build list of libraries
id: get_critical_libraries
run: |
run: |
LIBRARIES=$(grep -Po "^https://github.com/\K.+" ./CRITICAL_NOIR_LIBRARIES | jq -R -s -c 'split("\n") | map(select(. != "")) | map({ repo: ., path: ""})')
echo "libraries=$LIBRARIES"
echo "libraries=$LIBRARIES" >> $GITHUB_OUTPUT
Expand Down Expand Up @@ -593,14 +593,19 @@ jobs:
# Github actions seems to not expand "**" in globs by default.
shopt -s globstar
sed -i '/^compiler_version/d' ./**/Nargo.toml

- name: Run nargo test
working-directory: ./test-repo/${{ matrix.project.path }}
run: |
nargo test --silence-warnings --skip-brillig-constraints-check --format json ${{ matrix.project.nargo_args }} | tee ${{ github.workspace }}/noir-repo/.github/critical_libraries_status/${{ matrix.project.repo }}/${{ matrix.project.path }}.actual.jsonl
output_file=${{ github.workspace }}/noir-repo/.github/critical_libraries_status/${{ matrix.project.repo }}/${{ matrix.project.path }}.actual.jsonl
nargo test --silence-warnings --skip-brillig-constraints-check --format json ${{ matrix.project.nargo_args }} | tee $output_file
if [ ! -s $output_file ]; then
# The file is empty so we delete it to signal that `nargo test` failed before it could run any tests
rm -f $output_file
fi
env:
NARGO_IGNORE_TEST_FAILURES_FROM_FOREIGN_CALLS: true

- name: Compare test results
working-directory: ./noir-repo
run: .github/scripts/check_test_results.sh .github/critical_libraries_status/${{ matrix.project.repo }}/${{ matrix.project.path }}.failures.jsonl .github/critical_libraries_status/${{ matrix.project.repo }}/${{ matrix.project.path }}.actual.jsonl
Expand Down
2 changes: 1 addition & 1 deletion noir/noir-repo/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
6 changes: 3 additions & 3 deletions noir/noir-repo/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 All @@ -170,10 +169,11 @@ fn optimize_all(builder: SsaBuilder, options: &SsaEvaluatorOptions) -> Result<Ss
"Unrolling",
)?
.run_pass(Ssa::simplify_cfg, "Simplifying (2nd)")
.run_pass(Ssa::mem2reg, "Mem2Reg (2nd)")
.run_pass(Ssa::flatten_cfg, "Flattening")
.run_pass(Ssa::remove_bit_shifts, "After Removing Bit Shifts")
.run_pass(Ssa::remove_bit_shifts, "Removing Bit Shifts")
// Run mem2reg once more with the flattened CFG to catch any remaining loads/stores
.run_pass(Ssa::mem2reg, "Mem2Reg (2nd)")
.run_pass(Ssa::mem2reg, "Mem2Reg (3rd)")
// Run the inlining pass again to handle functions with `InlineType::NoPredicates`.
// Before flattening is run, we treat functions marked with the `InlineType::NoPredicates` as an entry point.
// This pass must come immediately following `mem2reg` as the succeeding passes
Expand Down
10 changes: 8 additions & 2 deletions noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/dfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -416,10 +416,15 @@ impl DataFlowGraph {
pub(crate) fn get_value_max_num_bits(&self, value: ValueId) -> u32 {
match self[value] {
Value::Instruction { instruction, .. } => {
let value_bit_size = self.type_of_value(value).bit_size();
if let Instruction::Cast(original_value, _) = self[instruction] {
self.type_of_value(original_value).bit_size()
let original_bit_size = self.type_of_value(original_value).bit_size();
// We might have cast e.g. `u1` to `u8` to be able to do arithmetic,
// in which case we want to recover the original smaller bit size;
// OTOH if we cast down, then we don't need the higher original size.
value_bit_size.min(original_bit_size)
} else {
self.type_of_value(value).bit_size()
value_bit_size
}
}

Expand Down Expand Up @@ -465,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
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,9 @@ impl Instruction {
// removed entirely.
Noop => true,

// Cast instructions can always be deduplicated
Cast(_, _) => true,

// Arrays can be mutated in unconstrained code so code that handles this case must
// take care to track whether the array was possibly mutated or not before
// deduplicating. Since we don't know if the containing pass checks for this, we
Expand All @@ -484,7 +487,6 @@ impl Instruction {
// with one that was disabled. See
// https://github.com/noir-lang/noir/pull/4716#issuecomment-2047846328.
Binary(_)
| Cast(_, _)
| Not(_)
| Truncate { .. }
| IfElse { .. }
Expand Down
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
Loading
Loading