Skip to content

perf: cache successor params to avoid O(P×A) cloning in arg demotion #7556

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.

Sign up for GitHub

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

Open
Fibonacci747 wants to merge 1 commit into
base: master
Choose a base branch
from
+2 −1
Open

perf: cache successor params to avoid O(P×A) cloning in arg demotion #7556

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion sway-ir/src/optimize/arg_demotion.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,9 +280,10 @@ fn demote_block_signature(context: &mut Context, function: &Function, block: Blo

let preds = block.pred_iter(context).copied().collect::<Vec<Block>>();
for pred in preds {
let params = pred.get_succ_params(context, &block);
for (arg_idx, _arg_val, arg_var) in &arg_vars {
// Get the value which is being passed to the block at this index.
let arg_val = pred.get_succ_params(context, &block)[*arg_idx];
let arg_val = params[*arg_idx];
Copy link
Copy Markdown

@cursor cursor bot Feb 16, 2026

Choose a reason for hiding this comment

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

Cached params causes silent no-op replacement for duplicate values

Medium Severity

When the same Value is passed at multiple demotable argument positions from the same predecessor (e.g., br block(V, V)), caching get_succ_params before the inner loop causes incorrect behavior. The first iteration's replace_values replaces ALL occurrences of V in the terminator with get_local_val_0. On the next iteration, replace_values with {V → get_local_val_1} becomes a silent no-op since V no longer exists in the terminator. The result is the terminator passing the wrong GetLocal pointer for subsequent duplicate positions.

Fix in Cursor Fix in Web


// Insert a `get_local` and `store` for each candidate argument and insert them at the
// end of this block, before the terminator.
Expand Down
Loading