chore(ACIR): try to use constant expressions in memory ops#10068
chore(ACIR): try to use constant expressions in memory ops#10068
Conversation
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: 3b9ed50 | Previous: 2e78193 | Ratio |
|---|---|---|---|
semaphore-depth-10 |
0.051 s |
0.042 s |
1.21 |
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: 2401086 | Previous: dc79733 | Ratio |
|---|---|---|---|
perfectly_parallel_batch_inversion_opcodes |
2779293 ns/iter (± 3670) |
2257462 ns/iter (± 6119) |
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: 2401086 | Previous: dc79733 | Ratio |
|---|---|---|---|
private-kernel-reset |
9.55 s |
7.544 s |
1.27 |
This comment was automatically generated by workflow using github-action-benchmark.
CC: @TomAFrench
|
Hmm... "Reports / Circuit sizes" always fails so there must be something wrong in this PR.. |
It is most likely failing when we try to serialize to barretenberg. You will have to re-run locally to see which tests are breaking. Although it is surprising to me that just using constants would fail as a similar experiment was done in #9970 (comment) and I was able to call into the backend. |
|
I finally managed to install Given that this is just cosmetic, I'll close this PR. |
Description
Problem
No issue, just trying to simplify the generated ACIR so it's easier to understand.
Summary
I'm actually not sure this is a valid change. However, all tests pass for me locally. I think it does lead to shorter ACIR, especially for slice operations.
Additional Context
Documentation
Check one:
PR Checklist
cargo fmton default settings.