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(ssa): Add flag to DIE pass to be able to keep store instructions #7106

Merged
merged 2 commits into from
Jan 17, 2025

Conversation

aakoshh
Copy link
Contributor

@aakoshh aakoshh commented Jan 17, 2025

Description

Problem*

Resolves #7104

Summary*

Adds a flattened flag to Function::dead_instruction_elimination which is passed to Instruction::can_eliminate_if_unused to prevent this assumption from removing the store instruction before the inlining, the flattening of the CFG, and mem2reg passes have happened. Thanks @vezenovm for the pointer!

Additional Context

Originally I thought I would have to track &mut block parameters as "use", but the assumptions listed in can_eliminate_in_unused suggested that store must be removed from ACIR, and to stop that from happening it's enough to give it more information, so it doesn't infer whether other passes happened already based on the number of blocks in the function.

It looked like tracking &mut parameters as "use" would be a bigger change, since only instruction results are checked for use at the moment. Maybe there are other cases which would better be handled that way?

For the use case of #7072 I thought we'd just call the function with false and be done with it.

Documentation*

Check one:

  • No documentation needed.
  • Documentation included in this PR.
  • [For Experimental Features] Documentation to be submitted in a separate PR.

PR Checklist*

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

@aakoshh aakoshh changed the title fix(ssa): Add flag to DIE pass to be able to keep store instructions feat(ssa): Add flag to DIE pass to be able to keep store instructions Jan 17, 2025
@aakoshh aakoshh requested a review from a team January 17, 2025 15:13
@aakoshh aakoshh force-pushed the 7104-fix-die-mut-ref-param branch from 408dbf4 to fc36f52 Compare January 17, 2025 15:17
Copy link
Contributor

github-actions bot commented Jan 17, 2025

Execution Report

Program Execution Time %
sha256_regression 0.051s 0%
regression_4709 0.001s 0%
ram_blowup_regression 0.598s 0%
rollup-root 0.104s 0%
rollup-merge 0.006s 0%
rollup-block-root 36.400s 0%
rollup-block-merge 0.107s 1%
rollup-base-public 1.224s 0%
rollup-base-private 0.451s 0%
private-kernel-tail 0.019s 0%
private-kernel-reset 0.310s 0%
private-kernel-inner 0.068s 0%

Copy link
Contributor

github-actions bot commented Jan 17, 2025

Compilation Report

Program Compilation Time %
sha256_regression 1.060s 6%
regression_4709 0.883s 11%
ram_blowup_regression 16.600s 4%
rollup-root 3.650s -4%
rollup-merge 2.026s -8%
rollup-block-root-single-tx 143.000s 2%
rollup-block-root-empty 2.118s -3%
rollup-block-root 136.000s -3%
rollup-block-merge 3.572s -3%
rollup-base-public 27.760s -2%
rollup-base-private 10.280s 3%
private-kernel-tail 1.025s 4%
private-kernel-reset 5.934s 0%
private-kernel-inner 2.106s 0%

Copy link
Member

@TomAFrench TomAFrench left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@TomAFrench TomAFrench enabled auto-merge January 17, 2025 15:25
@TomAFrench TomAFrench added this pull request to the merge queue Jan 17, 2025
Copy link
Contributor

github-actions bot commented Jan 17, 2025

Compilation Memory Report

Program Peak Memory
keccak256 77.65M
workspace 123.87M
regression_4709 424.15M
ram_blowup_regression 1.46G
rollup-root 601.26M
rollup-merge 494.24M
rollup-block-root-single-tx 16.06G
rollup-block-root-empty 488.41M
rollup-block-root 16.07G
rollup-block-merge 601.26M
rollup-base-public 2.38G
rollup-base-private 1.14G
private-kernel-tail 207.43M
private-kernel-reset 584.49M
private-kernel-inner 294.68M

Copy link
Contributor

github-actions bot commented Jan 17, 2025

Execution Memory Report

Program Peak Memory
keccak256 74.71M
workspace 123.58M
regression_4709 316.02M
ram_blowup_regression 512.62M
rollup-root 498.31M
rollup-merge 473.05M
rollup-block-root 1.22G
rollup-block-merge 498.33M
rollup-base-public 734.18M
rollup-base-private 590.51M
private-kernel-tail 180.91M
private-kernel-reset 245.52M
private-kernel-inner 208.92M

Merged via the queue into master with commit ed12ad7 Jan 17, 2025
101 checks passed
@TomAFrench TomAFrench deleted the 7104-fix-die-mut-ref-param branch January 17, 2025 15:37
AztecBot added a commit to AztecProtocol/aztec-packages that referenced this pull request Jan 18, 2025
…oir-lang/noir#7100)

feat: Parser and formatter support for `enum`s (noir-lang/noir#7110)
feat(brillig): SSA globals code gen (noir-lang/noir#7021)
feat: `loop` keyword in runtime and comptime code (noir-lang/noir#7096)
chore: Add benchmarking dashboard (noir-lang/noir#7068)
feat(experimental): try to infer lambda argument types inside calls (noir-lang/noir#7088)
feat(ssa): Add flag to DIE pass to be able to keep `store` instructions (noir-lang/noir#7106)
chore: Cookbook Onboard integration (noir-lang/noir#7044)
chore: lock to ubuntu 22.04 (noir-lang/noir#7098)
fix: Remove unused brillig functions (noir-lang/noir#7102)
chore(ssa): Use correct prefix when printing array values in global space (noir-lang/noir#7095)
AztecBot added a commit to AztecProtocol/aztec-packages that referenced this pull request Jan 18, 2025
…oir#7100)

feat: Parser and formatter support for `enum`s (noir-lang/noir#7110)
feat(brillig): SSA globals code gen (noir-lang/noir#7021)
feat: `loop` keyword in runtime and comptime code (noir-lang/noir#7096)
chore: Add benchmarking dashboard (noir-lang/noir#7068)
feat(experimental): try to infer lambda argument types inside calls (noir-lang/noir#7088)
feat(ssa): Add flag to DIE pass to be able to keep `store` instructions (noir-lang/noir#7106)
chore: Cookbook Onboard integration (noir-lang/noir#7044)
chore: lock to ubuntu 22.04 (noir-lang/noir#7098)
fix: Remove unused brillig functions (noir-lang/noir#7102)
chore(ssa): Use correct prefix when printing array values in global space (noir-lang/noir#7095)
AztecBot added a commit to AztecProtocol/aztec-packages that referenced this pull request Jan 19, 2025
…oir-lang/noir#7100)

feat: Parser and formatter support for `enum`s (noir-lang/noir#7110)
feat(brillig): SSA globals code gen (noir-lang/noir#7021)
feat: `loop` keyword in runtime and comptime code (noir-lang/noir#7096)
chore: Add benchmarking dashboard (noir-lang/noir#7068)
feat(experimental): try to infer lambda argument types inside calls (noir-lang/noir#7088)
feat(ssa): Add flag to DIE pass to be able to keep `store` instructions (noir-lang/noir#7106)
chore: Cookbook Onboard integration (noir-lang/noir#7044)
chore: lock to ubuntu 22.04 (noir-lang/noir#7098)
fix: Remove unused brillig functions (noir-lang/noir#7102)
chore(ssa): Use correct prefix when printing array values in global space (noir-lang/noir#7095)
AztecBot added a commit to AztecProtocol/aztec-packages that referenced this pull request Jan 19, 2025
…oir#7100)

feat: Parser and formatter support for `enum`s (noir-lang/noir#7110)
feat(brillig): SSA globals code gen (noir-lang/noir#7021)
feat: `loop` keyword in runtime and comptime code (noir-lang/noir#7096)
chore: Add benchmarking dashboard (noir-lang/noir#7068)
feat(experimental): try to infer lambda argument types inside calls (noir-lang/noir#7088)
feat(ssa): Add flag to DIE pass to be able to keep `store` instructions (noir-lang/noir#7106)
chore: Cookbook Onboard integration (noir-lang/noir#7044)
chore: lock to ubuntu 22.04 (noir-lang/noir#7098)
fix: Remove unused brillig functions (noir-lang/noir#7102)
chore(ssa): Use correct prefix when printing array values in global space (noir-lang/noir#7095)
AztecBot added a commit to AztecProtocol/aztec-packages that referenced this pull request Jan 20, 2025
…oir-lang/noir#7100)

feat: Parser and formatter support for `enum`s (noir-lang/noir#7110)
feat(brillig): SSA globals code gen (noir-lang/noir#7021)
feat: `loop` keyword in runtime and comptime code (noir-lang/noir#7096)
chore: Add benchmarking dashboard (noir-lang/noir#7068)
feat(experimental): try to infer lambda argument types inside calls (noir-lang/noir#7088)
feat(ssa): Add flag to DIE pass to be able to keep `store` instructions (noir-lang/noir#7106)
chore: Cookbook Onboard integration (noir-lang/noir#7044)
chore: lock to ubuntu 22.04 (noir-lang/noir#7098)
fix: Remove unused brillig functions (noir-lang/noir#7102)
chore(ssa): Use correct prefix when printing array values in global space (noir-lang/noir#7095)
AztecBot added a commit to AztecProtocol/aztec-packages that referenced this pull request Jan 20, 2025
…oir#7100)

feat: Parser and formatter support for `enum`s (noir-lang/noir#7110)
feat(brillig): SSA globals code gen (noir-lang/noir#7021)
feat: `loop` keyword in runtime and comptime code (noir-lang/noir#7096)
chore: Add benchmarking dashboard (noir-lang/noir#7068)
feat(experimental): try to infer lambda argument types inside calls (noir-lang/noir#7088)
feat(ssa): Add flag to DIE pass to be able to keep `store` instructions (noir-lang/noir#7106)
chore: Cookbook Onboard integration (noir-lang/noir#7044)
chore: lock to ubuntu 22.04 (noir-lang/noir#7098)
fix: Remove unused brillig functions (noir-lang/noir#7102)
chore(ssa): Use correct prefix when printing array values in global space (noir-lang/noir#7095)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Dead Instruction Elimination doesn't consider &mut parameters
3 participants