fix: remove saturation from loop bound increments#10479
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'Test Suite Duration'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.
| Benchmark suite | Current: 171b136 | Previous: 62a4432 | Ratio |
|---|---|---|---|
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_blob |
473 s |
371 s |
1.27 |
test_report_zkpassport_noir-ecdsa_ |
3 s |
2 s |
1.50 |
test_report_zkpassport_noir_rsa_ |
1 s |
0 s |
+∞ |
This comment was automatically generated by workflow using github-action-benchmark.
CC: @TomAFrench
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'Execution Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.
| Benchmark suite | Current: ff2361c | Previous: fd27764 | Ratio |
|---|---|---|---|
rollup-checkpoint-merge |
0.004 s |
0.003 s |
1.33 |
This comment was automatically generated by workflow using github-action-benchmark.
CC: @TomAFrench
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'ACVM Benchmarks'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.
| Benchmark suite | Current: 171b136 | Previous: 62a4432 | Ratio |
|---|---|---|---|
perfectly_parallel_batch_inversion_opcodes |
2783867 ns/iter (± 6575) |
2264195 ns/iter (± 1976) |
1.23 |
This comment was automatically generated by workflow using github-action-benchmark.
CC: @TomAFrench
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'Compilation Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.
| Benchmark suite | Current: 171b136 | Previous: 62a4432 | Ratio |
|---|---|---|---|
rollup-checkpoint-root |
541 s |
409 s |
1.32 |
This comment was automatically generated by workflow using github-action-benchmark.
CC: @TomAFrench
Automated pull of nightly from the [noir](https://github.com/noir-lang/noir) programming language, a dependency of Aztec. BEGIN_COMMIT_OVERRIDE chore: remove `local_annotations` from flattening (noir-lang/noir#10483) chore: better error recovery for multiple mut in pattern (noir-lang/noir#10490) chore(frontend): Tuple pattern tests and remove confusing arity error (noir-lang/noir#10480) chore: monomorphizer public fields (noir-lang/noir#9979) chore: remove a bunch of dummy definitions (noir-lang/noir#10482) feat(ssa): Limit the number of steps executed by the SSA interpreter during constant folding (noir-lang/noir#10481) fix: remove saturation from loop bound increments (noir-lang/noir#10479) fix(print): Print enums (noir-lang/noir#10472) fix(frontend): No negative overflow when quoting signed integer (noir-lang/noir#10331) chore: green light Brillig for audit (noir-lang/noir#10376) END_COMMIT_OVERRIDE
2 participants
Description
Problem
Resolves
Summary
This PR modifies how we increment/decrement loop
IntegerConstants so that we throw an ICE rather than saturating in the case of overflow. This should not have any significant impact as the only case where this can occur is when the upper bound isi128::MAXoru128::MAX.Previously the saturation behaviour would result in the lower and upper bounds being equal but we will now reject any loops where the upper bound is one of these two values with an ICE.
Additional Context
User Documentation
Check one:
PR Checklist
cargo fmton default settings.