Make run_on_operation return a boolean indicating that the IR has been changed.

Signed-off-by: Tomas Fabrizio Orsi <[email protected]>

Author: lima-limon-inc

dialects/hir/src/transforms/spill.rs

@@ -31,22 +31,22 @@ impl Pass for TransformSpills {
         &mut self,
         op: EntityMut<'_, Self::Target>,
         state: &mut PassExecutionState,
-    ) -> Result<(), Report> {
+    ) -> Result<bool, Report> {
         let function = op.into_entity_ref();
         log::debug!(target: "insert-spills", "computing and inserting spills for {}", function.as_operation());
 
         if function.is_declaration() {
             log::debug!(target: "insert-spills", "function has no body, no spills needed!");
             state.preserved_analyses_mut().preserve_all();
-            return Ok(());
+            return Ok(false);
         }
         let mut analysis =
             state.analysis_manager().get_analysis_for::<SpillAnalysis, Function>()?;
 
         if !analysis.has_spills() {
             log::debug!(target: "insert-spills", "no spills needed!");
             state.preserved_analyses_mut().preserve_all();
-            return Ok(());
+            return Ok(false);
         }
 
         // Take ownership of the spills analysis so that we can mutate the analysis state during

dialects/scf/src/transforms/cfg_to_scf.rs

@@ -51,7 +51,7 @@ impl Pass for LiftControlFlowToSCF {
         &mut self,
         op: EntityMut<'_, Self::Target>,
         state: &mut PassExecutionState,
-    ) -> Result<(), Report> {
+    ) -> Result<bool, Report> {
         let mut transformation = ControlFlowToSCFTransformation;
         let mut changed = false;
 
@@ -130,7 +130,8 @@ impl Pass for LiftControlFlowToSCF {
         });
 
         if result.was_interrupted() {
-            return result.into_result();
+            // TODO: Maybe Change into_result implementation
+            return result.into_result().map(|_| false);
         }
 
         log::debug!(
@@ -141,7 +142,7 @@ impl Pass for LiftControlFlowToSCF {
             state.preserved_analyses_mut().preserve_all();
         }
 
-        Ok(())
+        Ok(changed)
     }
 }
 

hir-transform/src/canonicalization.rs

@@ -93,10 +93,10 @@ impl Pass for Canonicalizer {
         &mut self,
         op: EntityMut<'_, Self::Target>,
         state: &mut PassExecutionState,
-    ) -> Result<(), Report> {
+    ) -> Result<bool, Report> {
         let Some(rewrites) = self.rewrites.as_ref() else {
             log::debug!("skipping canonicalization as there are no rewrite patterns to apply");
-            return Ok(());
+            return Ok(false);
         };
         let op = {
             let ptr = op.as_operation_ref();
@@ -129,16 +129,20 @@ impl Pass for Canonicalizer {
         }
 
         let op = op.borrow();
-        match converged {
+        let changed = match converged {
             Ok(changed) => {
-                log::debug!("canonicalization converged for '{}', changed={changed}", op.name())
+                log::debug!("canonicalization converged for '{}', changed={changed}", op.name());
+                changed
             }
-            Err(changed) => log::warn!(
-                "canonicalization failed to converge for '{}', changed={changed}",
-                op.name()
-            ),
-        }
+            Err(changed) => {
+                log::warn!(
+                    "canonicalization failed to converge for '{}', changed={changed}",
+                    op.name()
+                );
+                changed
+            }
+        };
 
-        Ok(())
+        Ok(changed)
     }
 }

hir-transform/src/sccp.rs

@@ -38,7 +38,7 @@ impl Pass for SparseConditionalConstantPropagation {
         &mut self,
         mut op: EntityMut<'_, Self::Target>,
         state: &mut PassExecutionState,
-    ) -> Result<(), Report> {
+    ) -> Result<bool, Report> {
         // Run sparse constant propagation + dead code analysis
         let mut solver = DataFlowSolver::default();
         solver.load::<DeadCodeAnalysis>();
@@ -58,7 +58,7 @@ impl SparseConditionalConstantPropagation {
         op: &mut Operation,
         _state: &mut PassExecutionState,
         solver: &DataFlowSolver,
-    ) -> Result<(), Report> {
+    ) -> Result<bool, Report> {
         let mut worklist = SmallVec::<[BlockRef; 8]>::default();
 
         let add_to_worklist = |regions: &RegionList, worklist: &mut SmallVec<[BlockRef; 8]>| {
@@ -76,6 +76,7 @@ impl SparseConditionalConstantPropagation {
 
         add_to_worklist(op.regions(), &mut worklist);
 
+        let mut replaced_any = false;
         while let Some(mut block) = worklist.pop() {
             let mut block = block.borrow_mut();
             let body = block.body_mut();
@@ -91,8 +92,10 @@ impl SparseConditionalConstantPropagation {
                 let mut replaced_all = num_results != 0;
                 for index in 0..num_results {
                     let result = { op.borrow().get_result(index).borrow().as_value_ref() };
-                    replaced_all &=
-                        replace_with_constant(solver, &mut builder, &mut folder, result);
+                    let replaced = replace_with_constant(solver, &mut builder, &mut folder, result);
+
+                    replaced_any |= replaced;
+                    replaced_all &= replaced;
                 }
 
                 // If all of the results of the operation were replaced, try to erase the operation
@@ -112,7 +115,7 @@ impl SparseConditionalConstantPropagation {
             builder.set_insertion_point_to_start(block.as_block_ref());
 
             for arg in block.arguments() {
-                replace_with_constant(
+                replaced_any |= replace_with_constant(
                     solver,
                     &mut builder,
                     &mut folder,
@@ -121,7 +124,7 @@ impl SparseConditionalConstantPropagation {
             }
         }
 
-        Ok(())
+        Ok(replaced_any)
     }
 }
 

hir-transform/src/sink.rs

@@ -96,7 +96,7 @@ impl Pass for ControlFlowSink {
         &mut self,
         op: EntityMut<'_, Self::Target>,
         state: &mut PassExecutionState,
-    ) -> Result<(), Report> {
+    ) -> Result<bool, Report> {
         let op = op.into_entity_ref();
         log::debug!(target: "control-flow-sink", "sinking operations in {op}");
 
@@ -105,6 +105,7 @@ impl Pass for ControlFlowSink {
 
         let dominfo = state.analysis_manager().get_analysis::<DominanceInfo>()?;
 
+        let mut sunk = false;
         operation.raw_prewalk_all::<Forward, _>(|op: OperationRef| {
             let regions_to_sink = {
                 let op = op.borrow();
@@ -118,7 +119,7 @@ impl Pass for ControlFlowSink {
             };
 
             // Sink side-effect free operations.
-            control_flow_sink(
+            sunk = control_flow_sink(
                 &regions_to_sink,
                 &dominfo,
                 |op: &Operation, _region: &Region| op.is_memory_effect_free(),
@@ -132,7 +133,7 @@ impl Pass for ControlFlowSink {
             );
         });
 
-        Ok(())
+        Ok(sunk)
     }
 }
 
@@ -172,7 +173,7 @@ impl Pass for SinkOperandDefs {
         &mut self,
         op: EntityMut<'_, Self::Target>,
         _state: &mut PassExecutionState,
-    ) -> Result<(), Report> {
+    ) -> Result<bool, Report> {
         let operation = op.as_operation_ref();
         drop(op);
 
@@ -184,6 +185,7 @@ impl Pass for SinkOperandDefs {
         // then process the worklist, moving everything into position.
         let mut worklist = alloc::collections::VecDeque::default();
 
+        let mut changed = false;
         // Visit ops in "true" post-order (i.e. block bodies are visited bottom-up).
         operation.raw_postwalk_all::<Backward, _>(|operation: OperationRef| {
             // Determine if any of this operation's operands represent one of the following:
@@ -308,6 +310,7 @@ impl Pass for SinkOperandDefs {
                         log::trace!(target: "sink-operand-defs", "    rewriting operand {operand_value} as {replacement}");
                         operand.borrow_mut().set(replacement);
 
+                        changed = true;
                         // If no other uses of this value remain, then remove the original
                         // operation, as it is now dead.
                         if !operand_value.borrow().is_used() {
@@ -354,6 +357,7 @@ impl Pass for SinkOperandDefs {
                         log::trace!(target: "sink-operand-defs", "    rewriting operand {operand_value} as {replacement}");
                         sink_state.replacements.insert(operand_value, replacement);
                         operand.borrow_mut().set(replacement);
+                        changed = true;
                     } else {
                         log::trace!(target: "sink-operand-defs", "    defining op is a constant with no other uses, moving into place");
                         // The original op can be moved
@@ -397,7 +401,7 @@ impl Pass for SinkOperandDefs {
             }
         }
 
-        Ok(())
+        Ok(changed)
     }
 }
 
@@ -548,12 +552,14 @@ pub fn control_flow_sink<P, F>(
     dominfo: &DominanceInfo,
     should_move_into_region: P,
     move_into_region: F,
-) where
+) -> bool
+where
     P: Fn(&Operation, &Region) -> bool,
     F: Fn(OperationRef, RegionRef),
 {
     let sinker = Sinker::new(dominfo, should_move_into_region, move_into_region);
-    sinker.sink_regions(regions);
+    let sunk_regions = sinker.sink_regions(regions);
+    sunk_regions > 0
 }
 
 /// Populates `regions` with regions of the provided region branch op that are executed at most once

hir-transform/src/spill.rs

@@ -113,7 +113,7 @@ pub fn transform_spills(
     analysis: &mut SpillAnalysis,
     interface: &mut dyn TransformSpillsInterface,
     analysis_manager: AnalysisManager,
-) -> Result<(), Report> {
+) -> Result<bool, Report> {
     assert!(
         op.borrow().implements::<dyn SingleRegion>(),
         "the spills transformation is not supported when the root op is multi-region"
@@ -252,7 +252,8 @@ pub fn transform_spills(
         )?;
     }
 
-    Ok(())
+    // QUESTION: If the code has reached this point, then, in theory, it must've gotten changed. Right? Requires double check.
+    Ok(true)
 }
 
 fn rewrite_single_block_spills(

hir/src/pass.rs

@@ -157,10 +157,10 @@ impl Pass for Print {
         &mut self,
         op: crate::EntityMut<'_, Self::Target>,
         _state: &mut PassExecutionState,
-    ) -> Result<(), crate::Report> {
+    ) -> Result<bool, crate::Report> {
         let op = op.into_entity_ref();
         self.print_ir(op);
-        Ok(())
+        Ok(false)
     }
 }
 

hir/src/pass/manager.rs

@@ -956,9 +956,9 @@ impl OpToOpPassAdaptor {
 
         let mut result =
             if let Some(adaptor) = pass.as_any_mut().downcast_mut::<OpToOpPassAdaptor>() {
-                adaptor.run_on_operation(op, &mut execution_state, verify)
+                adaptor.run_on_operation(op, &mut execution_state, verify).map(|_| ())
             } else {
-                pass.run_on_operation(op, &mut execution_state)
+                pass.run_on_operation(op, &mut execution_state).map(|_| ())
             };
 
         // Invalidate any non-preserved analyses
@@ -1006,7 +1006,7 @@ impl OpToOpPassAdaptor {
         op: OperationRef,
         state: &mut PassExecutionState,
         verify: bool,
-    ) -> Result<(), Report> {
+    ) -> Result<bool, Report> {
         let analysis_manager = state.analysis_manager();
         let instrumentor = analysis_manager.pass_instrumentor();
         let parent_info = PipelineParentInfo {
@@ -1041,7 +1041,7 @@ impl OpToOpPassAdaptor {
             }
         }
 
-        Ok(())
+        Ok(false)
     }
 }
 
@@ -1078,7 +1078,7 @@ impl Pass for OpToOpPassAdaptor {
         &mut self,
         _op: EntityMut<'_, Operation>,
         _state: &mut PassExecutionState,
-    ) -> Result<(), Report> {
+    ) -> Result<bool, Report> {
         unreachable!("unexpected call to `Pass::run_on_operation` for OpToOpPassAdaptor")
     }
 }

hir/src/pass/pass.rs

@@ -44,7 +44,7 @@ pub trait OperationPass {
         &mut self,
         op: OperationRef,
         state: &mut PassExecutionState,
-    ) -> Result<(), Report>;
+    ) -> Result<bool, Report>;
     fn run_pipeline(
         &mut self,
         pipeline: &mut OpPassManager,
@@ -121,7 +121,7 @@ where
         &mut self,
         mut op: OperationRef,
         state: &mut PassExecutionState,
-    ) -> Result<(), Report> {
+    ) -> Result<bool, Report> {
         let op = <<P as Pass>::Target as PassTarget>::into_target_mut(&mut op);
         <P as Pass>::run_on_operation(self, op, state)
     }
@@ -231,7 +231,7 @@ pub trait Pass: Sized + Any {
         &mut self,
         op: EntityMut<'_, Self::Target>,
         state: &mut PassExecutionState,
-    ) -> Result<(), Report>;
+    ) -> Result<bool, Report>;
     /// Schedule an arbitrary pass pipeline on the provided operation.
     ///
     /// This can be invoke any time in a pass to dynamic schedule more passes. The provided
@@ -330,7 +330,7 @@ where
         &mut self,
         op: EntityMut<'_, Self::Target>,
         state: &mut PassExecutionState,
-    ) -> Result<(), Report> {
+    ) -> Result<bool, Report> {
         (**self).run_on_operation(op, state)
     }