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(mem2reg): Remove trivial stores #5865

Closed
wants to merge 27 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
e0ad0a9
remove trivial stores
vezenovm Aug 29, 2024
5bbaca8
fmt
vezenovm Aug 29, 2024
a66009a
don't include arrays in last store
vezenovm Aug 29, 2024
ce31cb0
switch to using get_known_value and set_known_value
vezenovm Aug 30, 2024
45db90b
add from_rc flag to store
vezenovm Aug 30, 2024
96b70bd
cargo fmt
vezenovm Aug 30, 2024
6a0f8c3
Merge branch 'master' into mv/simplify-immediate-stores
vezenovm Aug 30, 2024
a595349
Merge branch 'master' into mv/simplify-immediate-stores
vezenovm Aug 30, 2024
82bc2ea
don't call mem2reg and die again to fix uhashmap
vezenovm Aug 30, 2024
6ce03eb
cargo fmt
vezenovm Aug 30, 2024
5c41302
comment out extra mem2reg and die
vezenovm Aug 30, 2024
568c083
remove last_loads approach and add some comments
vezenovm Aug 30, 2024
c1c2b61
cleanup
vezenovm Aug 30, 2024
6222963
Merge branch 'master' into mv/simplify-immediate-stores
vezenovm Aug 30, 2024
6ead23d
Merge branch 'master' into mv/simplify-immediate-stores
vezenovm Sep 3, 2024
dae5a44
cleanup
vezenovm Sep 3, 2024
e920768
Merge remote-tracking branch 'origin/mv/simplify-immediate-stores' in…
vezenovm Sep 3, 2024
4962db1
reduce diff
vezenovm Sep 3, 2024
31224eb
missed push
vezenovm Sep 3, 2024
adacab4
cleanup
vezenovm Sep 3, 2024
ea39ead
Merge branch 'master' into mv/simplify-immediate-stores
vezenovm Sep 3, 2024
20d20af
remove from_rc from Store and track in mem2reg
vezenovm Sep 3, 2024
2d8f5e6
remove unnecessary
vezenovm Sep 3, 2024
90a4fbe
move rc reload state tracking
vezenovm Sep 3, 2024
90d5715
improve handling of rc reload state and handle call when tracking
vezenovm Sep 4, 2024
96ea083
Merge branch 'master' into mv/simplify-immediate-stores
vezenovm Sep 4, 2024
0db5610
master merge
vezenovm Sep 4, 2024
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
5 changes: 5 additions & 0 deletions compiler/noirc_evaluator/src/ssa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,11 @@ pub(crate) fn optimize_into_acir(
.run_pass(Ssa::remove_enable_side_effects, "After EnableSideEffectsIf removal:")
.run_pass(Ssa::fold_constants_using_constraints, "After Constraint Folding:")
.run_pass(Ssa::dead_instruction_elimination, "After Dead Instruction Elimination:")
// TODO: mem2reg and DIE were put here to delete any stores that can be removes following die removing loads.
// Decide whether we want to run mem2reg once more or just have it able to handle this case on its own
.run_pass(Ssa::mem2reg, "After Mem2Reg:")
// The extra mem2reg requires another DIE as we can have allocates we do not want
.run_pass(Ssa::dead_instruction_elimination, "After Dead Instruction Elimination:")
.run_pass(Ssa::array_set_optimization, "After Array Set Optimizations:")
.finish();

Expand Down
8 changes: 7 additions & 1 deletion compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
//!
//! Repeating this algorithm for each block in the function in program order should result in
//! optimizing out most known loads. However, identifying all aliases correctly has been proven
//! undecidable in general (Landi, 1992). So this pass will not always optimize out all loads

Check warning on line 59 in compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (Landi)
//! that could theoretically be optimized out. This pass can be performed at any time in the
//! SSA optimization pipeline, although it will be more successful the simpler the program's CFG is.
//! This pass is currently performed several times to enable other passes - most notably being
Expand Down Expand Up @@ -111,7 +111,7 @@
/// Load and Store instructions that should be removed at the end of the pass.
///
/// We avoid removing individual instructions as we go since removing elements
/// from the middle of Vecs many times will be slower than a single call to `retain`.

Check warning on line 114 in compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (Vecs)
instructions_to_remove: BTreeSet<InstructionId>,

/// Track a value's last load across all blocks.
Expand Down Expand Up @@ -253,7 +253,6 @@

// If the load is known, replace it with the known value and remove the load
if let Some(value) = references.get_known_value(address) {
let result = self.inserter.function.dfg.instruction_results(instruction)[0];
self.inserter.map_value(result, value);
self.instructions_to_remove.insert(instruction);
} else {
Expand All @@ -274,6 +273,13 @@
self.instructions_to_remove.insert(*last_store);
}

if let Some(last_load) = self.last_loads.get(&address) {
let load_result = self.inserter.function.dfg.instruction_results(*last_load)[0];
if load_result == value {
self.instructions_to_remove.insert(instruction);
}
}
jfecher marked this conversation as resolved.
Show resolved Hide resolved

references.set_known_value(address, value);
references.last_stores.insert(address, instruction);
}
Expand Down
3 changes: 3 additions & 0 deletions compiler/noirc_evaluator/src/ssa/opt/mem2reg/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,9 @@ impl Block {
// known references if this reference is ever stored to.
}
}
// else {
// self.references.insert(result, ReferenceValue::Known(address));
// }
jfecher marked this conversation as resolved.
Show resolved Hide resolved
}

/// Iterate through each known alias of the given address and apply the function `f` to each.
Expand Down
Loading