Skip to content

Fix memcpyopt to allow multiple passes of optimisations#7657

Merged
xunilrj merged 14 commits into
masterfrom
xunilrj/opt-multiple-passes
Jun 17, 2026
Merged

Fix memcpyopt to allow multiple passes of optimisations#7657
xunilrj merged 14 commits into
masterfrom
xunilrj/opt-multiple-passes

Conversation

@xunilrj

@xunilrj xunilrj commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Description

This PR is another prelude to #7646.

We discovered that memcpyopt does not work when we run it multiple times. The cause is that we bail the optimisation when a symbol has more than one write. This is correct, but we were only considering 'store'.

Now we are correctly also considering all other ways to write into "symbols".

Checklist

  • I have linked to any relevant issues.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have updated the documentation where relevant (API docs, the reference, and the Sway book).
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added (or requested a maintainer to add) the necessary Breaking* or New Feature labels where relevant.
  • I have done my best to ensure that my PR adheres to the Fuel Labs Code Review Standards.
  • I have requested a review from the relevant team or maintainers.

@xunilrj xunilrj requested a review from a team as a code owner June 15, 2026 23:11
@xunilrj xunilrj force-pushed the xunilrj/opt-multiple-passes branch from 77233e1 to cac6c9c Compare June 16, 2026 00:42
@xunilrj xunilrj temporarily deployed to fuel-sway-bot June 16, 2026 00:42 — with GitHub Actions Inactive
@xunilrj xunilrj requested review from a team as code owners June 16, 2026 01:02
@xunilrj xunilrj temporarily deployed to fuel-sway-bot June 16, 2026 01:03 — with GitHub Actions Inactive
@xunilrj xunilrj temporarily deployed to fuel-sway-bot June 16, 2026 15:32 — with GitHub Actions Inactive
@xunilrj xunilrj temporarily deployed to fuel-sway-bot June 16, 2026 16:32 — with GitHub Actions Inactive
@xunilrj xunilrj temporarily deployed to fuel-sway-bot June 16, 2026 16:35 — with GitHub Actions Inactive
@xunilrj

xunilrj commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

@cursor BugBot run

@cursor

cursor Bot commented Jun 16, 2026

Copy link
Copy Markdown

Here's my BugBot analysis of PR #7657:

CI Status: All green. 34 checks passed, 0 failed. The only pending item is policy-bot: master (a review policy check, not a code issue). 5 checks were skipped (publish-related, only triggered on merge).

Local Build & Test Results:

  • cargo build --workspace — compiles successfully
  • cargo test -p sway-ir — all 37 tests pass (18 unit + 19 integration including memcpyopt and memcpy_prop)
  • cargo clippy -p sway-ir — only 1 pre-existing minor warning (unrelated to this PR)
  • cargo test -p sway-core --lib — all 20 tests pass

Code Review Summary:

The changes are sound. The two key modifications are:

  1. memcpyopt.rs — The stores_map now correctly tracks all write instructions (Store, MemCopyVal, MemClearVal, MemCopyBytes), not just Store. This fixes the bug where multiple passes of the optimization would fail because the "single write" check didn't account for MemCopy/MemClear operations that are themselves produced by prior optimization passes.

  2. pass_manager.rs — The pass group now runs in a loop (up to 2 iterations), re-running all passes if any pass made modifications. This enables cascading optimizations that only become available after a first pass transforms the IR. The early exit when nothing is modified ensures no performance regression for code that converges in one pass.

No bugs or issues found. The PR is ready for human review/approval.

Open in Web Open in Cursor 

@xunilrj xunilrj enabled auto-merge (squash) June 16, 2026 17:59
@ironcev ironcev added compiler General compiler. Should eventually become more specific as the issue is triaged compiler: ir IRgen and sway-ir including optimization passes labels Jun 17, 2026
@ironcev

ironcev commented Jun 17, 2026

Copy link
Copy Markdown
Member

👍

ironcev
ironcev previously approved these changes Jun 17, 2026

@ironcev ironcev left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Great to see those gas reductions in tests :-) Two nitpicks in comments for retaining initial clarity, otherwise 👍 👍

Comment thread sway-ir/src/optimize/memcpyopt.rs Outdated
Comment thread sway-ir/src/optimize/memcpyopt.rs Outdated
@xunilrj xunilrj merged commit 493e7b7 into master Jun 17, 2026
40 checks passed
@xunilrj xunilrj deleted the xunilrj/opt-multiple-passes branch June 17, 2026 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

compiler: ir IRgen and sway-ir including optimization passes compiler General compiler. Should eventually become more specific as the issue is triaged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants