From d33bda784badc871a073ba577b08bfd44b642547 Mon Sep 17 00:00:00 2001
From: Russel Arbore <russel.jma@gmail.com>
Date: Tue, 26 Mar 2024 18:42:50 -0500
Subject: [PATCH] Comment changes in forkify and phi_elim
---
hercules_ir/Cargo.toml | 2 +-
hercules_opt/Cargo.toml | 2 +-
hercules_opt/src/forkify.rs | 4 ++-
hercules_opt/src/phi_elim.rs | 68 +++++++++++++++++++-----------------
4 files changed, 41 insertions(+), 35 deletions(-)
diff --git a/hercules_ir/Cargo.toml b/hercules_ir/Cargo.toml
index b68e6b24..39fbebe5 100644
--- a/hercules_ir/Cargo.toml
+++ b/hercules_ir/Cargo.toml
@@ -1,7 +1,7 @@
[package]
name = "hercules_ir"
version = "0.1.0"
-authors = ["Russel Arbore <rarbore2@illinois.edu>"]
+authors = ["Russel Arbore <rarbore2@illinois.edu>, Aaron Councilman <aaronjc4@illinois.edu>"]
[dependencies]
rand = "*"
diff --git a/hercules_opt/Cargo.toml b/hercules_opt/Cargo.toml
index 7743a73f..5cf76b09 100644
--- a/hercules_opt/Cargo.toml
+++ b/hercules_opt/Cargo.toml
@@ -1,7 +1,7 @@
[package]
name = "hercules_opt"
version = "0.1.0"
-authors = ["Russel Arbore <rarbore2@illinois.edu>"]
+authors = ["Russel Arbore <rarbore2@illinois.edu>, Aaron Councilman <aaronjc4@illinois.edu>"]
[dependencies]
ordered-float = "*"
diff --git a/hercules_opt/src/forkify.rs b/hercules_opt/src/forkify.rs
index 2e58e38f..32e812a9 100644
--- a/hercules_opt/src/forkify.rs
+++ b/hercules_opt/src/forkify.rs
@@ -17,7 +17,9 @@ pub fn forkify(
def_use: &ImmutableDefUseMap,
loops: &LoopTree,
) {
- // Ignore loops that are already fork-joins.
+ // Ignore loops that are already fork-joins. TODO: re-calculate def_use per
+ // loop, since it's technically invalidated after each individual loop
+ // modification.
let natural_loops = loops
.bottom_up_loops()
.into_iter()
diff --git a/hercules_opt/src/phi_elim.rs b/hercules_opt/src/phi_elim.rs
index 16f61423..21a1a2e5 100644
--- a/hercules_opt/src/phi_elim.rs
+++ b/hercules_opt/src/phi_elim.rs
@@ -1,43 +1,44 @@
-/* A Hercules IR transformation that
+extern crate hercules_ir;
+
+use std::collections::HashMap;
+
+use self::hercules_ir::get_uses_mut;
+use self::hercules_ir::ir::*;
+
+/*
+ * This is a Hercules IR transformation that:
* - Eliminates phi nodes where all inputs are the same (here this means the
- * same node in IR, we are not performing GVM, SCCP, or any similar
- * optimization)
- * - Eliminate regions with only a single predecessor
+ * same node in IR, GVN and related are separate).
+ * - Eliminate regions with only a single predecessor.
*
* The first of these optimizations is inspired by the description of the SSA
* construction algorithm in Braun et al., CC 2013 used by the front-end. This
* optimization performs the phi removal suggested by that paper, as in our
- * construction algorithm performing it during code generation is difficult.
+ * construction algorithm performing it during IR code generation is difficult.
*/
-extern crate hercules_ir;
-
-use std::collections::HashMap;
-
-use self::hercules_ir::ir::*;
-use self::hercules_ir::get_uses_mut;
/*
- * Top level function to run phi elimination, as described above.
- * Deletes nodes by setting nodes to gravestones. Works with a function already
- * containing gravestones.
+ * Top level function to run phi elimination, as described above. Deletes nodes
+ * by setting nodes to gravestones. Works with a function already containing
+ * gravestones.
*/
-pub fn phi_elim(function : &mut Function) {
+pub fn phi_elim(function: &mut Function) {
// Keep a map of nodes that we need to replace, and what we need to replace
- // them with
- let mut replace_nodes : HashMap<usize, NodeID> = HashMap::new();
+ // them with.
+ let mut replace_nodes: HashMap<usize, NodeID> = HashMap::new();
- // Iterate over the nodes of the function until convergence.
- // In this loop, we look for phis and regions that can be eliminated, mark
- // them as gravestones, and add them to the replacement map. For all other
- // nodes, we see if any of their arguments are in the replacement map and
- // if so eliminate them
+ // Iterate over the nodes of the function until convergence. In this loop,
+ // we look for phis and regions that can be eliminated, mark them as
+ // gravestones, and add them to the replacement map. For all other nodes, we
+ // see if any of their arguments are in the replacement map - if so,
+ // eliminate them.
let mut changed = true;
while changed {
changed = false;
for (idx, node) in function.nodes.iter_mut().enumerate() {
// Replace any nodes that this node uses that are in the replacement
- // map
+ // map.
for u in get_uses_mut(node).as_mut() {
let old_id = u.idx();
if let Some(replacement) = replace_nodes.get(&old_id) {
@@ -46,31 +47,34 @@ pub fn phi_elim(function : &mut Function) {
}
}
- // Then, check if this node can be removed
- if let Node::Phi { control : _, data } = node {
- // For a phi, we can remove it if all of its data inputs are
- // the same node or self-cycles
+ // Then, check if this node can be removed.
+ if let Node::Phi { control: _, data } = node {
+ // For a phi, we can remove it if all of its data inputs are the
+ // same node or self-cycles.
let mut unique = Some(data[0]);
for i in 1..data.len() {
- // Ignore self-loops
- if data[i].idx() != idx && Some(data[i]) != unique{
+ // Ignore self-loops.
+ if data[i].idx() != idx && Some(data[i]) != unique {
if unique.unwrap().idx() == idx {
unique = Some(data[i]);
} else {
- unique = None; break;
+ unique = None;
+ break;
}
}
}
if let Some(value) = unique {
changed = true;
replace_nodes.insert(idx, value);
- *node = Node::Start; // mark node as removed
+ // Delete this node.
+ *node = Node::Start;
}
} else if let Node::Region { preds } = node {
if preds.len() == 1 {
changed = true;
replace_nodes.insert(idx, preds[0]);
- *node = Node::Start; // mark as dead
+ // Delete this node.
+ *node = Node::Start;
}
}
}
--
GitLab