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: allow deduplicating Truncate and Not instructions #7010

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

TomAFrench
Copy link
Member

Description

Problem*

Resolves

Summary*

These two instructions are not sensitive to EnableSideEffectsIf instructions so we can safely deduplicate them

Additional Context

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.

@TomAFrench TomAFrench requested a review from a team January 9, 2025 18:00
@TomAFrench
Copy link
Member Author

huh

@TomAFrench TomAFrench removed the request for review from a team January 9, 2025 18:03
@TomAFrench TomAFrench marked this pull request as draft January 9, 2025 18:03
Copy link
Contributor

github-actions bot commented Jan 9, 2025

Execution Memory Report

Program Peak Memory
keccak256 74.66M
workspace 123.82M
regression_4709 315.99M
ram_blowup_regression 512.41M
rollup-base-public 482.46M
rollup-base-private 328.58M
private-kernel-tail 180.93M
private-kernel-reset 249.60M
private-kernel-inner 209.77M

Copy link
Contributor

github-actions bot commented Jan 9, 2025

Compilation Memory Report

Program Peak Memory
keccak256 78.11M
workspace 123.75M
regression_4709 422.96M
ram_blowup_regression 1.58G
rollup-base-public 4.85G
rollup-base-private 1.26G
private-kernel-tail 207.08M
private-kernel-reset 730.50M
private-kernel-inner 294.30M

Copy link
Contributor

github-actions bot commented Jan 9, 2025

Compilation Report

Program Compilation Time %
sha256_regression 1.298s 3%
regression_4709 0.792s 1%
ram_blowup_regression 18.240s 0%
rollup-base-public 40.000s 2%
rollup-base-private 12.840s 0%
private-kernel-tail 1.035s 0%

Copy link
Contributor

github-actions bot commented Jan 9, 2025

Execution Report

Program Execution Time %
sha256_regression 0.055s 1%
regression_4709 0.001s 0%
ram_blowup_regression 0.574s -1%
rollup-base-public 1.240s 0%
rollup-base-private 0.470s 0%
private-kernel-tail 0.021s 0%

@TomAFrench TomAFrench marked this pull request as ready for review January 10, 2025 12:06
@TomAFrench TomAFrench requested a review from a team January 10, 2025 12:06
Copy link
Contributor

@jfecher jfecher left a comment

Choose a reason for hiding this comment

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

We are already deduplicating these instructions since requires_acir_gen_predicate returns false for them unless I'm missing something

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants