From 2fb14138d415be3f36797ee2fdfd9146aa1e39a0 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Wed, 12 Jul 2023 14:54:49 +0900 Subject: [PATCH] [rust] Port EliminateRedundantPhis This is a nearly 1:1 port of EliminateRedundantPhis to Rust, the algorithm is identical and all differences are superficial. There are few things missing (an invariant instead of a panic in one place, recursing into function expressions) but the Rust version is still going to end up shorter despite keeping all the comments. --- compiler/forget/Cargo.lock | 10 ++ compiler/forget/Cargo.toml | 1 + .../crates/fixtures/tests/fixtures_test.rs | 5 +- ...res_test__fixtures@ssa-reassign-if.js.snap | 7 +- compiler/forget/crates/hir-ssa/Cargo.toml | 1 + .../hir-ssa/src/eliminate_redundant_phis.rs | 96 +++++++++++++++ compiler/forget/crates/hir-ssa/src/lib.rs | 4 +- .../crates/hir-ssa/src/minimize_phis.rs | 5 - compiler/forget/crates/utils/Cargo.toml | 9 ++ compiler/forget/crates/utils/src/lib.rs | 113 ++++++++++++++++++ 10 files changed, 237 insertions(+), 14 deletions(-) create mode 100644 compiler/forget/crates/hir-ssa/src/eliminate_redundant_phis.rs delete mode 100644 compiler/forget/crates/hir-ssa/src/minimize_phis.rs create mode 100644 compiler/forget/crates/utils/Cargo.toml create mode 100644 compiler/forget/crates/utils/src/lib.rs diff --git a/compiler/forget/Cargo.lock b/compiler/forget/Cargo.lock index 444fb5cd57..56b8310dea 100644 --- a/compiler/forget/Cargo.lock +++ b/compiler/forget/Cargo.lock @@ -504,6 +504,7 @@ dependencies = [ "estree", "estree-swc", "hir", + "hir-ssa", "insta", "miette 5.9.0", ] @@ -644,8 +645,10 @@ version = "0.1.0" dependencies = [ "bumpalo", "hir", + "indexmap 2.0.0", "miette 5.9.0", "thiserror", + "utils", ] [[package]] @@ -2782,6 +2785,13 @@ dependencies = [ "percent-encoding", ] +[[package]] +name = "utils" +version = "0.1.0" +dependencies = [ + "bumpalo", +] + [[package]] name = "vergen" version = "7.5.1" diff --git a/compiler/forget/Cargo.toml b/compiler/forget/Cargo.toml index 03a9ac1feb..42a921df89 100644 --- a/compiler/forget/Cargo.toml +++ b/compiler/forget/Cargo.toml @@ -9,6 +9,7 @@ members = [ "crates/estree", "crates/estree-codegen", "crates/estree-swc", + "crates/utils" ] # Make insta run faster by compiling with release mode optimizations diff --git a/compiler/forget/crates/fixtures/tests/fixtures_test.rs b/compiler/forget/crates/fixtures/tests/fixtures_test.rs index adcbcb1265..24a3132045 100644 --- a/compiler/forget/crates/fixtures/tests/fixtures_test.rs +++ b/compiler/forget/crates/fixtures/tests/fixtures_test.rs @@ -1,11 +1,11 @@ -use std::fmt::Write; +use std::{env, fmt::Write}; use build_hir::build; use bumpalo::Bump; use estree::{ModuleItem, Statement}; use estree_swc::parse; use hir::{Environment, Print, Registry}; -use hir_ssa::enter_ssa; +use hir_ssa::{eliminate_redundant_phis, enter_ssa}; use insta::{assert_snapshot, glob}; use miette::{NamedSource, Report}; @@ -34,6 +34,7 @@ fn fixtures() { match build(&environment, *fun) { Ok(mut fun) => { enter_ssa(&environment, &mut fun).unwrap(); + eliminate_redundant_phis(&environment, &mut fun); fun.print(&fun.body, &mut output).unwrap(); } Err(error) => { diff --git a/compiler/forget/crates/fixtures/tests/snapshots/fixtures_test__fixtures@ssa-reassign-if.js.snap b/compiler/forget/crates/fixtures/tests/snapshots/fixtures_test__fixtures@ssa-reassign-if.js.snap index b3d238e947..a48c570161 100644 --- a/compiler/forget/crates/fixtures/tests/snapshots/fixtures_test__fixtures@ssa-reassign-if.js.snap +++ b/compiler/forget/crates/fixtures/tests/snapshots/fixtures_test__fixtures@ssa-reassign-if.js.snap @@ -54,8 +54,6 @@ bb5 (block) [16] Goto bb3 bb3 (block) predecessors: bb4, bb5 - x$15: phi(bb4: x$10, bb5: x$10) - y$17: phi(bb4: y$8, bb5: y$8) z$19: phi(bb4: z$11, bb5: z$12) [17] Goto bb1 bb6 (block) @@ -65,11 +63,10 @@ bb6 (block) [20] Goto bb1 bb1 (block) predecessors: bb3, bb6 - x$14: phi(bb3: x$15, bb6: x$13) - y$16: phi(bb3: y$17, bb6: y$8) + x$14: phi(bb3: x$10, bb6: x$13) z$18: phi(bb3: z$19, bb6: z$9) [21] #15 = LoadLocal unknown x$14 - [22] #16 = LoadLocal unknown y$16 + [22] #16 = LoadLocal unknown y$8 [23] #17 = Binary unknown #15 + unknown #16 [24] #18 = LoadLocal unknown z$18 [25] #19 = Binary unknown #17 + unknown #18 diff --git a/compiler/forget/crates/hir-ssa/Cargo.toml b/compiler/forget/crates/hir-ssa/Cargo.toml index da0aa7f361..b08d68fcf9 100644 --- a/compiler/forget/crates/hir-ssa/Cargo.toml +++ b/compiler/forget/crates/hir-ssa/Cargo.toml @@ -7,6 +7,7 @@ edition = "2021" [dependencies] hir = { path = "../hir" } +utils = { path = "../utils" } bumpalo = "3.13.0" indexmap = "2.0.0" miette = { version = "5.9.0" } diff --git a/compiler/forget/crates/hir-ssa/src/eliminate_redundant_phis.rs b/compiler/forget/crates/hir-ssa/src/eliminate_redundant_phis.rs new file mode 100644 index 0000000000..50b06cbf69 --- /dev/null +++ b/compiler/forget/crates/hir-ssa/src/eliminate_redundant_phis.rs @@ -0,0 +1,96 @@ +use std::collections::{HashMap, HashSet}; + +use hir::{BlockId, Environment, Function, Identifier, IdentifierId, HIR}; +use utils::RetainMut; + +/// Pass to eliminate redundant phi nodes: +/// all operands are the same identifier, ie `x2 = phi(x1, x1, x1)`. +/// all operands are the same identifier *or* the output of the phi, ie `x2 = phi(x1, x2, x1, x2)`. +/// +/// In both these cases, the phi is eliminated and all usages of the phi identifier +/// are replaced with the other operand (ie in both cases above, all usages of `x2` are replaced with `x1` . +/// +/// The algorithm is inspired by that in https://pp.info.uni-karlsruhe.de/uploads/publikationen/braun13cc.pdf +/// but modified to reduce passes over the CFG. We visit the blocks in reverse postorder. Each time a redundant +/// phi is encountered we add a mapping (eg x2 -> x1) to a rewrite table. Subsequent instructions, terminals, +/// and phis rewrite all their identifiers based on this table. The algorithm loops over the CFG repeatedly +/// until there are no new rewrites: for a CFG without back-edges it completes in a single pass. +type Rewrites<'a> = HashMap>; +pub fn eliminate_redundant_phis<'a>(_env: &'a Environment, fun: &mut Function<'a>) { + let hir = &mut fun.body; + let mut rewrites = Rewrites::new(); + + let mut has_back_edge = false; + let mut visited = HashSet::::new(); + + let mut len; + loop { + len = rewrites.len(); + + for (_, block) in hir.blocks.iter_mut() { + if !has_back_edge { + for predecessor in block.predecessors.iter() { + if !visited.contains(predecessor) { + has_back_edge = true; + } + } + } + visited.insert(block.id); + + block.phis.retain_mut(|phi| { + // Remap operands in case they are from eliminated phis + for (_, operand) in phi.operands.iter_mut() { + rewrite(&rewrites, operand); + } + // Find if the phi can be eliminated + let mut rewrite: Option = None; + for (_, operand) in phi.operands.iter() { + if operand.id == phi.identifier.id { + // This operand is the same as the phi itself + continue; + } + match &rewrite { + Some(rewrite) if rewrite.id == operand.id => { + // this operand is the same as the other operands + continue; + } + Some(_) => { + // There are multiple operands not equal to the phi itself, + // the phi cannot be eliminated (true to retain the phi) + return true; + } + None => { + rewrite = Some(operand.clone()); + } + } + } + rewrites.insert(phi.identifier.id, rewrite.unwrap()); + // The phi can be eliminated (false to not retain) + false + }); + + for instr_ix in block.instructions.iter() { + let instr = &mut hir.instructions[usize::from(*instr_ix)]; + instr.each_identifier_store(|store| { + rewrite(&rewrites, &mut store.identifier.identifier) + }); + instr.each_identifier_load(|load| rewrite(&rewrites, &mut load.identifier)); + } + } + + // We only need to loop if there were newly eliminated phis in this iteration + // *and* the CFG has loops. If there are no loops then all eliminated phis must + // have been propagated forwards since we visit in RPO + if has_back_edge && rewrites.len() > len { + continue; + } else { + break; + } + } +} + +fn rewrite<'a>(rewrites: &Rewrites<'a>, identifier: &mut Identifier<'a>) { + if let Some(rewrite) = rewrites.get(&identifier.id) { + *identifier = rewrite.clone() + } +} diff --git a/compiler/forget/crates/hir-ssa/src/lib.rs b/compiler/forget/crates/hir-ssa/src/lib.rs index 2618141cdd..b5119bdd7b 100644 --- a/compiler/forget/crates/hir-ssa/src/lib.rs +++ b/compiler/forget/crates/hir-ssa/src/lib.rs @@ -1,7 +1,7 @@ +mod eliminate_redundant_phis; mod enter; mod leave; -mod minimize_phis; +pub use eliminate_redundant_phis::eliminate_redundant_phis; pub use enter::enter_ssa; pub use leave::leave_ssa; -pub use minimize_phis::minimize_phis; diff --git a/compiler/forget/crates/hir-ssa/src/minimize_phis.rs b/compiler/forget/crates/hir-ssa/src/minimize_phis.rs deleted file mode 100644 index 05db37903f..0000000000 --- a/compiler/forget/crates/hir-ssa/src/minimize_phis.rs +++ /dev/null @@ -1,5 +0,0 @@ -use hir::{Environment, HIR}; - -pub fn minimize_phis<'a>(_env: &'a Environment, _hir: &mut HIR<'a>) { - todo!("minimize_phis()"); -} diff --git a/compiler/forget/crates/utils/Cargo.toml b/compiler/forget/crates/utils/Cargo.toml new file mode 100644 index 0000000000..476d93c1a2 --- /dev/null +++ b/compiler/forget/crates/utils/Cargo.toml @@ -0,0 +1,9 @@ +[package] +name = "utils" +version = "0.1.0" +edition = "2021" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] +bumpalo = { version = "3.13.0", features = ["boxed", "collections"] } \ No newline at end of file diff --git a/compiler/forget/crates/utils/src/lib.rs b/compiler/forget/crates/utils/src/lib.rs new file mode 100644 index 0000000000..174e1c3a3a --- /dev/null +++ b/compiler/forget/crates/utils/src/lib.rs @@ -0,0 +1,113 @@ +use bumpalo::collections::Vec; + +pub trait RetainMut { + fn retain_mut(&mut self, f: F) -> () + where + F: FnMut(&mut T) -> bool; +} + +impl<'a, T> RetainMut for Vec<'a, T> { + fn retain_mut(&mut self, mut f: F) -> () + where + F: FnMut(&mut T) -> bool, + { + // NOTE: implementation adapted from retain_mut crate + // which is in turn adapted from Rust stdlib + // https://docs.rs/retain_mut/latest/src/retain_mut/lib.rs.html#68-69 + + let original_len = self.len(); + // Avoid double drop if the drop guard is not executed, + // since we may make some holes during the process. + unsafe { self.set_len(0) }; + + // Vec: [Kept, Kept, Hole, Hole, Hole, Hole, Unchecked, Unchecked] + // |<- processed len ->| ^- next to check + // |<- deleted cnt ->| + // |<- original_len ->| + // Kept: Elements which predicate returns true on. + // Hole: Moved or dropped element slot. + // Unchecked: Unchecked valid elements. + // + // This drop guard will be invoked when predicate or `drop` of element panicked. + // It shifts unchecked elements to cover holes and `set_len` to the correct length. + // In cases when predicate and `drop` never panick, it will be optimized out. + struct BackshiftOnDrop<'a, 'b, T> { + v: &'b mut Vec<'a, T>, + processed_len: usize, + deleted_cnt: usize, + original_len: usize, + } + + impl Drop for BackshiftOnDrop<'_, '_, T> { + fn drop(&mut self) { + if self.deleted_cnt > 0 { + // SAFETY: Trailing unchecked items must be valid since we never touch them. + unsafe { + std::ptr::copy( + self.v.as_ptr().add(self.processed_len), + self.v + .as_mut_ptr() + .add(self.processed_len - self.deleted_cnt), + self.original_len - self.processed_len, + ); + } + } + // SAFETY: After filling holes, all items are in contiguous memory. + unsafe { + self.v.set_len(self.original_len - self.deleted_cnt); + } + } + } + + let mut g = BackshiftOnDrop { + v: self, + processed_len: 0, + deleted_cnt: 0, + original_len, + }; + + fn process_loop( + original_len: usize, + f: &mut F, + g: &mut BackshiftOnDrop<'_, '_, T>, + ) where + F: FnMut(&mut T) -> bool, + { + while g.processed_len != original_len { + // SAFETY: Unchecked element must be valid. + let cur = unsafe { &mut *g.v.as_mut_ptr().add(g.processed_len) }; + if !f(cur) { + // Advance early to avoid double drop if `drop_in_place` panicked. + g.processed_len += 1; + g.deleted_cnt += 1; + // SAFETY: We never touch this element again after dropped. + unsafe { std::ptr::drop_in_place(cur) }; + // We already advanced the counter. + if DELETED { + continue; + } else { + break; + } + } + if DELETED { + // SAFETY: `deleted_cnt` > 0, so the hole slot must not overlap with current element. + // We use copy for move, and never touch this element again. + unsafe { + let hole_slot = g.v.as_mut_ptr().add(g.processed_len - g.deleted_cnt); + std::ptr::copy_nonoverlapping(cur, hole_slot, 1); + } + } + g.processed_len += 1; + } + } + + // Stage 1: Nothing was deleted. + process_loop::(original_len, &mut f, &mut g); + + // Stage 2: Some elements were deleted. + process_loop::(original_len, &mut f, &mut g); + + // All item are processed. This can be optimized to `set_len` by LLVM. + drop(g); + } +}