BE-258: HashQL: Add Changed enum to track MIR transformation pass modifications (#8210)

Author: indietyp

.claude/skills/testing-hashql/SKILL.md

@@ -180,6 +180,12 @@ For testing MIR transformation and analysis passes directly with programmaticall
 - Edge cases requiring specific MIR structures hard to produce from source
 - Benchmarking pass performance
 
+**Key features:**
+
+- Transform passes return `Changed` enum (`Yes`, `No`, `Unknown`) to indicate modifications
+- Test harness captures and includes `Changed` value in snapshots for verification
+- Snapshot format: before MIR → `Changed: Yes/No/Unknown` separator → after MIR
+
 **Quick Example:**
 
 ```rust
@@ -201,10 +207,10 @@ builder
 let body = builder.finish(0, TypeBuilder::synthetic(&env).integer());
 ```
 
-📖 **Full Guide:** [resources/mir-builder-guide.md](resources/mir-builder-guide.md)
+📖 **Full Guide:** [references/mir-builder-guide.md](references/mir-builder-guide.md)
 
 ## References
 
-- [compiletest Guide](resources/compiletest-guide.md) - Detailed UI test documentation
-- [Testing Strategies](resources/testing-strategies.md) - Choosing the right approach
-- [MIR Builder Guide](resources/mir-builder-guide.md) - Programmatic MIR construction for tests
+- [compiletest Guide](references/compiletest-guide.md) - Detailed UI test documentation
+- [Testing Strategies](references/testing-strategies.md) - Choosing the right approach
+- [MIR Builder Guide](references/mir-builder-guide.md) - Programmatic MIR construction for tests

.claude/skills/testing-hashql/references/mir-builder-guide.md

@@ -228,10 +228,11 @@ builder.build_block(bb_merge).ret(x);  // x receives value from param
 
 ## Test Harness Pattern
 
-Standard pattern used across transform pass tests:
+Standard pattern used across transform pass tests. The harness captures and displays
+the `Changed` return value to verify pass behavior:
 
 ```rust
-use std::path::PathBuf;
+use std::{io::Write as _, path::PathBuf};
 use bstr::ByteVec as _;
 use hashql_core::{
     pretty::Formatter,
@@ -274,12 +275,16 @@ fn assert_pass<'heap>(
         .format(DefIdSlice::from_raw(&bodies), &[])
         .expect("should be able to write bodies");
 
-    text_format
-        .writer
-        .extend(b"\n\n------------------------------------\n\n");
-
-    // Run the pass
-    YourPass::new().run(context, &mut bodies[0]);
+    // Run the pass and capture change status
+    let changed = YourPass::new().run(context, &mut bodies[0]);
+    
+    // Include Changed value in snapshot for verification
+    write!(
+        text_format.writer,
+        "\n\n{:=^50}\n\n",
+        format!(" Changed: {changed:?} ")
+    )
+    .expect("infallible");
 
     // Format after
     text_format

libs/@local/hashql/compiletest/src/suite/mir_pass_transform_cfg_simplify.rs

@@ -14,7 +14,7 @@ use hashql_mir::{
     context::MirContext,
     def::{DefId, DefIdSlice, DefIdVec},
     intern::Interner,
-    pass::{TransformPass as _, transform::CfgSimplify},
+    pass::{Changed, TransformPass as _, transform::CfgSimplify},
 };
 
 use super::{RunContext, Suite, SuiteDiagnostic, common::process_issues, mir_reify::mir_reify};
@@ -63,7 +63,7 @@ pub(crate) fn mir_pass_transform_cfg_simplify<'heap>(
 
     let mut pass = CfgSimplify::new_in(&mut scratch);
     for body in bodies.as_mut_slice() {
-        pass.run(&mut context, body);
+        let _: Changed = pass.run(&mut context, body);
     }
 
     process_issues(diagnostics, context.diagnostics)?;

libs/@local/hashql/compiletest/src/suite/mir_pass_transform_dse.rs

@@ -11,7 +11,7 @@ use hashql_mir::{
     context::MirContext,
     def::{DefId, DefIdSlice, DefIdVec},
     intern::Interner,
-    pass::{TransformPass as _, transform::DeadStoreElimination},
+    pass::{Changed, TransformPass as _, transform::DeadStoreElimination},
 };
 
 use super::{
@@ -45,7 +45,7 @@ pub(crate) fn mir_pass_transform_dse<'heap>(
     // CFG -> SROA -> Inst -> DSE
     let mut pass = DeadStoreElimination::new_in(&mut scratch);
     for body in bodies.as_mut_slice() {
-        pass.run(&mut context, body);
+        let _: Changed = pass.run(&mut context, body);
     }
 
     process_issues(diagnostics, context.diagnostics)?;

libs/@local/hashql/compiletest/src/suite/mir_pass_transform_inst_simplify.rs

@@ -11,7 +11,7 @@ use hashql_mir::{
     context::MirContext,
     def::{DefId, DefIdSlice, DefIdVec},
     intern::Interner,
-    pass::{TransformPass as _, transform::InstSimplify},
+    pass::{Changed, TransformPass as _, transform::InstSimplify},
 };
 
 use super::{
@@ -44,7 +44,7 @@ pub(crate) fn mir_pass_transform_inst_simplify<'heap>(
 
     let mut pass = InstSimplify::new_in(&mut scratch);
     for body in bodies.as_mut_slice() {
-        pass.run(&mut context, body);
+        let _: Changed = pass.run(&mut context, body);
     }
 
     process_issues(diagnostics, context.diagnostics)?;

libs/@local/hashql/compiletest/src/suite/mir_pass_transform_sroa.rs

@@ -11,7 +11,7 @@ use hashql_mir::{
     context::MirContext,
     def::{DefId, DefIdSlice, DefIdVec},
     intern::Interner,
-    pass::{TransformPass as _, transform::Sroa},
+    pass::{Changed, TransformPass as _, transform::Sroa},
 };
 
 use super::{
@@ -44,7 +44,7 @@ pub(crate) fn mir_pass_transform_sroa<'heap>(
 
     let mut pass = Sroa::new_in(&mut scratch);
     for body in bodies.as_mut_slice() {
-        pass.run(&mut context, body);
+        let _: Changed = pass.run(&mut context, body);
     }
 
     process_issues(diagnostics, context.diagnostics)?;

libs/@local/hashql/mir/benches/transform.rs

@@ -5,7 +5,7 @@
     clippy::similar_names
 )]
 
-use core::hint::black_box;
+use core::{cmp, hint::black_box};
 
 use codspeed_criterion_compat::{BatchSize, Bencher, Criterion, criterion_group, criterion_main};
 use hashql_core::{
@@ -301,10 +301,10 @@ fn create_complex_cfg<'heap>(env: &Environment<'heap>, interner: &Interner<'heap
 
 #[expect(unsafe_code)]
 #[inline]
-fn run_bencher(
+fn run_bencher<T>(
     bencher: &mut Bencher,
     body: for<'heap> fn(&Environment<'heap>, &Interner<'heap>) -> Body<'heap>,
-    mut func: impl for<'env, 'heap> FnMut(&mut MirContext<'env, 'heap>, &mut Body<'heap>),
+    mut func: impl for<'env, 'heap> FnMut(&mut MirContext<'env, 'heap>, &mut Body<'heap>) -> T,
 ) {
     // NOTE: `heap` must not be moved or reassigned; `heap_ptr` assumes its address is stable
     // for the entire duration of this function.
@@ -348,8 +348,8 @@ fn run_bencher(
                 diagnostics: DiagnosticIssues::new(),
             };
 
-            func(black_box(&mut context), black_box(body));
-            context.diagnostics
+            let value = func(black_box(&mut context), black_box(body));
+            (context.diagnostics, value)
         },
         BatchSize::PerIteration,
     );
@@ -440,30 +440,45 @@ fn pipeline(criterion: &mut Criterion) {
         let mut scratch = Scratch::new();
 
         run_bencher(bencher, create_linear_cfg, |context, body| {
-            CfgSimplify::new_in(&mut scratch).run(context, body);
-            Sroa::new_in(&mut scratch).run(context, body);
-            InstSimplify::new().run(context, body);
-            DeadStoreElimination::new_in(&mut scratch).run(context, body);
+            let mut changed = CfgSimplify::new_in(&mut scratch).run(context, body);
+            changed = cmp::max(changed, Sroa::new_in(&mut scratch).run(context, body));
+            changed = cmp::max(changed, InstSimplify::new().run(context, body));
+            changed = cmp::max(
+                changed,
+                DeadStoreElimination::new_in(&mut scratch).run(context, body),
+            );
+
+            changed
         });
     });
     group.bench_function("diamond", |bencher| {
         let mut scratch = Scratch::new();
 
         run_bencher(bencher, create_diamond_cfg, |context, body| {
-            CfgSimplify::new_in(&mut scratch).run(context, body);
-            Sroa::new_in(&mut scratch).run(context, body);
-            InstSimplify::new().run(context, body);
-            DeadStoreElimination::new_in(&mut scratch).run(context, body);
+            let mut changed = CfgSimplify::new_in(&mut scratch).run(context, body);
+            changed = cmp::max(changed, Sroa::new_in(&mut scratch).run(context, body));
+            changed = cmp::max(changed, InstSimplify::new().run(context, body));
+            changed = cmp::max(
+                changed,
+                DeadStoreElimination::new_in(&mut scratch).run(context, body),
+            );
+
+            changed
         });
     });
     group.bench_function("complex", |bencher| {
         let mut scratch = Scratch::new();
 
         run_bencher(bencher, create_complex_cfg, |context, body| {
-            CfgSimplify::new_in(&mut scratch).run(context, body);
-            Sroa::new_in(&mut scratch).run(context, body);
-            InstSimplify::new().run(context, body);
-            DeadStoreElimination::new_in(&mut scratch).run(context, body);
+            let mut changed = CfgSimplify::new_in(&mut scratch).run(context, body);
+            changed = cmp::max(changed, Sroa::new_in(&mut scratch).run(context, body));
+            changed = cmp::max(changed, InstSimplify::new().run(context, body));
+            changed = cmp::max(
+                changed,
+                DeadStoreElimination::new_in(&mut scratch).run(context, body),
+            );
+
+            changed
         });
     });
 }

libs/@local/hashql/mir/src/pass/analysis/data_dependency/mod.rs

@@ -119,7 +119,7 @@ impl<'env, 'heap, A: Allocator + Clone> AnalysisPass<'env, 'heap>
 
         graph.derive(&body.local_decls, |id, _| id);
 
-        let Ok(()) = DataDependencyAnalysisVisitor {
+        Ok(()) = DataDependencyAnalysisVisitor {
             graph: &mut graph,
             constant_bindings: &mut constant_bindings,
             context,

libs/@local/hashql/mir/src/pass/mod.rs

@@ -58,6 +58,55 @@ const fn simplify_type_name(name: &'static str) -> &'static str {
     }
 }
 
+/// Indicates whether a pass modified the MIR.
+///
+/// Passes return this to signal whether they made changes, enabling the pass manager to skip
+/// dependent re-analyses when nothing changed.
+#[must_use]
+#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
+pub enum Changed {
+    /// The pass definitely made modifications.
+    Yes = 2,
+    /// The pass may have made modifications, but precise tracking was not possible.
+    Unknown = 1,
+    /// The pass made no modifications.
+    No = 0,
+}
+
+impl Changed {
+    /// Returns `true` if the MIR may have changed.
+    ///
+    /// This is the conservative choice: returns `true` for both [`Changed::Yes`] and
+    /// [`Changed::Unknown`], ensuring dependent analyses are invalidated when in doubt, but may
+    /// result in unnecessary passes to be run.
+    #[must_use]
+    pub const fn conservative(self) -> bool {
+        match self {
+            Self::Yes | Self::Unknown => true,
+            Self::No => false,
+        }
+    }
+
+    /// Returns `true` only if the MIR definitely changed.
+    ///
+    /// This is the optimistic choice: returns `true` only for [`Changed::Yes`], assuming
+    /// [`Changed::Unknown`] did not actually modify anything. Use with caution, as this may
+    /// skip necessary re-runs of passes.
+    #[must_use]
+    pub const fn optimistic(self) -> bool {
+        match self {
+            Self::Yes => true,
+            Self::Unknown | Self::No => false,
+        }
+    }
+}
+
+impl From<bool> for Changed {
+    fn from(value: bool) -> Self {
+        if value { Self::Yes } else { Self::No }
+    }
+}
+
 /// A transformation or analysis pass over MIR.
 ///
 /// Passes operate on a [`Body`] within a [`MirContext`], allowing them to read and modify the
@@ -86,7 +135,7 @@ pub trait TransformPass<'env, 'heap> {
     ///
     /// The `context` provides access to the heap allocator, type environment, interner, and
     /// diagnostic collection. The `body` can be read and modified in place.
-    fn run(&mut self, context: &mut MirContext<'env, 'heap>, body: &mut Body<'heap>);
+    fn run(&mut self, context: &mut MirContext<'env, 'heap>, body: &mut Body<'heap>) -> Changed;
 
     /// Returns a human-readable name for this pass.
     ///

libs/@local/hashql/mir/src/pass/transform/cfg_simplify/mod.rs

@@ -64,7 +64,7 @@ use crate::{
         terminator::{Goto, Terminator, TerminatorKind},
     },
     context::MirContext,
-    pass::TransformPass,
+    pass::{Changed, TransformPass},
 };
 
 /// Control-flow graph simplification pass.
@@ -479,8 +479,9 @@ impl<'env, 'heap, A: BumpAllocator> TransformPass<'env, 'heap> for CfgSimplify<A
     /// Uses a worklist algorithm that processes blocks in reverse postorder and re-enqueues
     /// predecessors when changes occur. This ensures optimization opportunities propagate
     /// backward through the CFG until a fixed point is reached.
-    fn run(&mut self, context: &mut MirContext<'env, 'heap>, body: &mut Body<'heap>) {
+    fn run(&mut self, context: &mut MirContext<'env, 'heap>, body: &mut Body<'heap>) -> Changed {
         let mut queue = WorkQueue::new(body.basic_blocks.len());
+        let mut changed = false;
 
         // Process in reverse of reverse-postorder (i.e., roughly postorder) to handle
         // successors before predecessors, maximizing optimization opportunities.
@@ -500,6 +501,7 @@ impl<'env, 'heap, A: BumpAllocator> TransformPass<'env, 'heap> for CfgSimplify<A
                 simplified = true;
             }
 
+            changed |= simplified;
             if !simplified {
                 continue;
             }
@@ -510,11 +512,22 @@ impl<'env, 'heap, A: BumpAllocator> TransformPass<'env, 'heap> for CfgSimplify<A
             }
         }
 
+        if !changed {
+            // We haven't made any changes, meaning that we can skip further analysis, as no blocks
+            // will be made unreachable, and SSA won't be violated.
+            return Changed::No;
+        }
+
         // Unreachable blocks will be dead, therefore must be removed
         let mut dbe = DeadBlockElimination::new_in(&mut self.alloc);
-        dbe.run(context, body);
+        let _: Changed = dbe.run(context, body);
 
         // Simplifications may break SSA (e.g., merged blocks with conflicting definitions).
-        SsaRepair::new_in(&mut self.alloc).run(context, body);
+        let _: Changed = SsaRepair::new_in(&mut self.alloc).run(context, body);
+
+        // We ignore the changed of the sub-passes above, because we **know** that we already
+        // modified, if they don't doesn't matter.
+
+        changed.into()
     }
 }