From 6bc8ec41824f50f7fb57dc1112736415042fe7c3 Mon Sep 17 00:00:00 2001
From: Russel Arbore <russel.jma@gmail.com>
Date: Sun, 24 Nov 2024 23:10:52 -0800
Subject: [PATCH] Fix inlining DC substitution
---
hercules_ir/src/ir.rs | 8 +++----
hercules_opt/src/editor.rs | 10 ++++++---
hercules_opt/src/inline.rs | 43 +++++++++++++++++++++++++++++++++++---
hercules_opt/src/pass.rs | 1 +
4 files changed, 52 insertions(+), 10 deletions(-)
diff --git a/hercules_ir/src/ir.rs b/hercules_ir/src/ir.rs
index ed340a76..c4ab9b3e 100644
--- a/hercules_ir/src/ir.rs
+++ b/hercules_ir/src/ir.rs
@@ -917,11 +917,11 @@ impl Index {
}
}
- pub fn lower_case_name(&self) -> &'static str {
+ pub fn lower_case_name(&self) -> String {
match self {
- Index::Field(_) => "field",
- Index::Variant(_) => "variant",
- Index::Position(_) => "position",
+ Index::Field(idx) => format!("field {}", idx),
+ Index::Variant(idx) => format!("variant {}", idx),
+ Index::Position(idxs) => format!("position {:?}", &*idxs),
}
}
}
diff --git a/hercules_opt/src/editor.rs b/hercules_opt/src/editor.rs
index a75ea38a..1981b129 100644
--- a/hercules_opt/src/editor.rs
+++ b/hercules_opt/src/editor.rs
@@ -244,12 +244,12 @@ impl<'a: 'b, 'b> FunctionEditor<'a> {
self.edits
}
- pub fn node_ids(&self) -> impl Iterator<Item = NodeID> {
+ pub fn node_ids(&self) -> impl ExactSizeIterator<Item = NodeID> {
let num = self.function.nodes.len();
(0..num).map(NodeID::new)
}
- pub fn dynamic_constant_ids(&self) -> impl Iterator<Item = DynamicConstantID> {
+ pub fn dynamic_constant_ids(&self) -> impl ExactSizeIterator<Item = DynamicConstantID> {
let num = self.dynamic_constants.borrow().len();
(0..num).map(DynamicConstantID::new)
}
@@ -272,6 +272,10 @@ impl<'a, 'b> FunctionEdit<'a, 'b> {
id.idx() >= self.editor.mutable_nodes.len() || self.editor.mutable_nodes[id.idx()]
}
+ pub fn num_dynamic_constants(&self) -> usize {
+ self.editor.dynamic_constants.borrow().len() + self.added_dynamic_constants.len()
+ }
+
pub fn add_node(&mut self, node: Node) -> NodeID {
let id = NodeID::new(self.editor.function.nodes.len() + self.added_nodeids.len());
// Added nodes need to have an entry in the def-use map.
@@ -312,7 +316,7 @@ impl<'a, 'b> FunctionEdit<'a, 'b> {
}
}
- pub fn replace_all_uses(mut self, old: NodeID, new: NodeID) -> Result<Self, Self> {
+ pub fn replace_all_uses(self, old: NodeID, new: NodeID) -> Result<Self, Self> {
self.replace_all_uses_where(old, new, |_| true)
}
diff --git a/hercules_opt/src/inline.rs b/hercules_opt/src/inline.rs
index 720717b0..6f6df573 100644
--- a/hercules_opt/src/inline.rs
+++ b/hercules_opt/src/inline.rs
@@ -200,8 +200,38 @@ fn inline_func(
|| node.is_dynamic_constant()
|| node.is_call()
{
- for (dc_a, dc_b) in zip(dcs_a, dcs_b.iter()) {
- substitute_dynamic_constants_in_node(*dc_a, *dc_b, &mut node, &mut edit);
+ // We have to perform the subsitution in two steps. First,
+ // we map every dynamic constant A to a non-sense dynamic
+ // constant ID. Second, we map each non-sense dynamic
+ // constant ID to the appropriate dynamic constant B. Why
+ // not just do this in one step from A to B? We update
+ // dynamic constants one at a time, so imagine the following
+ // A -> B mappings:
+ // ID 0 -> ID 1
+ // ID 1 -> ID 0
+ // First, we apply the first mapping. This changes all
+ // references to dynamic constant 0 to dynamic constant 1.
+ // Then, we apply the second mapping. This updates all
+ // already present references to dynamic constant 1, as well
+ // as the new references we just made in the first step. We
+ // actually want to institute all the updates
+ // *simultaneously*, hence the two step maneuver.
+ let num_dcs = edit.num_dynamic_constants();
+ for (dc_a, dc_n) in zip(dcs_a, num_dcs..) {
+ substitute_dynamic_constants_in_node(
+ *dc_a,
+ DynamicConstantID::new(dc_n),
+ &mut node,
+ &mut edit,
+ );
+ }
+ for (dc_n, dc_b) in zip(num_dcs.., dcs_b.iter()) {
+ substitute_dynamic_constants_in_node(
+ DynamicConstantID::new(dc_n),
+ *dc_b,
+ &mut node,
+ &mut edit,
+ );
}
}
let mut uses = get_uses_mut(&mut node);
@@ -267,6 +297,13 @@ fn substitute_dynamic_constants(
return dc_b;
}
+ // Since we substitute non-sense dynamic constant IDs earlier, we explicitly
+ // check that the provided ID to replace inside of is valid. Otherwise,
+ // ignore.
+ if dc_c.idx() >= edit.num_dynamic_constants() {
+ return dc_c;
+ }
+
// If C is not just A, look inside of it to possibly substitute a child DC.
let dc_clone = edit.get_dynamic_constant(dc_c).clone();
match dc_clone {
@@ -339,7 +376,7 @@ fn substitute_dynamic_constants_in_type(
.map(|field_id| substitute_dynamic_constants_in_type(dc_a, dc_b, *field_id, edit))
.collect();
if new_fields != *fields {
- edit.add_type(Type::Summation(new_fields))
+ edit.add_type(Type::Product(new_fields))
} else {
ty
}
diff --git a/hercules_opt/src/pass.rs b/hercules_opt/src/pass.rs
index f5e0a9ad..33d7bfb3 100644
--- a/hercules_opt/src/pass.rs
+++ b/hercules_opt/src/pass.rs
@@ -788,6 +788,7 @@ impl PassManager {
.expect("PANIC: Unable to write output module file contents.");
}
}
+ println!("Ran pass: {:?}", pass);
}
}
--
GitLab