fix: aggressive

Author: indietyp

libs/@local/hashql/mir/src/pass/transform/inline/find.rs

@@ -23,10 +23,8 @@ use crate::{
 ///
 /// A callsite is eligible if:
 /// - It's a direct call (function is a constant `FnPtr`).
-/// - Its target SCC has not already been inlined into this caller.
-///
-/// The SCC check prevents cycles: once we've inlined a function (or any function
-/// in its SCC) into a filter, we won't inline it again.
+/// - It's not a self-call.
+/// - Its target is not a loop breaker.
 pub(crate) struct FindCallsiteVisitor<'ctx, 'state, 'env, 'heap, A: Allocator> {
     /// The filter function we're finding callsites in.
     pub caller: DefId,
@@ -53,10 +51,10 @@ impl<'heap, A: Allocator> Visitor<'heap> for FindCallsiteVisitor<'_, '_, '_, 'he
             return Ok(());
         };
 
-        let target_component = self.state.components.scc(ptr);
-
-        // Skip if we've already inlined this SCC into this caller.
-        if self.state.inlined.contains(self.caller, target_component) {
+        // Skip self-calls and calls to loop breakers. Breakers are the cycle
+        // cut points: inlining them would reintroduce the recursion that
+        // breaker selection removed.
+        if ptr == self.caller || self.state.loop_breakers.contains(ptr) {
             return Ok(());
         }
 

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

@@ -33,7 +33,7 @@
 //! aggressive inlining to fully flatten the filter logic. The aggressive phase:
 //! 1. Iterates up to `aggressive_inline_cutoff` times per filter.
 //! 2. On each iteration, inlines all eligible callsites found in the filter.
-//! 3. Tracks which SCCs have been inlined to prevent cycles.
+//! 3. Calls to loop breakers and self-calls are skipped to prevent cycles.
 //! 4. Emits a diagnostic if the cutoff is reached.
 //!
 //! # Budget System
@@ -53,18 +53,12 @@ use alloc::collections::BinaryHeap;
 use core::{alloc::Allocator, cmp, mem};
 
 use hashql_core::{
-    graph::{
-        DirectedGraph as _,
-        algorithms::{
-            Tarjan, TriColorDepthFirstSearch,
-            tarjan::{Members, SccId, StronglyConnectedComponents},
-        },
+    graph::algorithms::{
+        Tarjan, TriColorDepthFirstSearch,
+        tarjan::{Members, SccId, StronglyConnectedComponents},
     },
     heap::{BumpAllocator, Heap},
-    id::{
-        Id as _, IdSlice,
-        bit_vec::{DenseBitSet, SparseBitMatrix},
-    },
+    id::{Id as _, IdSlice, bit_vec::DenseBitSet},
     span::SpanId,
 };
 
@@ -214,11 +208,6 @@ struct InlineState<'ctx, 'state, 'env, 'heap, A: Allocator> {
     /// Calls to a breaker within its SCC are skipped during inlining.
     /// Calls from a breaker to non-breakers are still inlined.
     loop_breakers: DenseBitSet<DefId>,
-    /// Tracks which SCCs have been inlined into each function.
-    ///
-    /// Used to prevent cycles during aggressive inlining: once an SCC
-    /// has been inlined into a filter, it won't be inlined again.
-    inlined: SparseBitMatrix<DefId, SccId, A>,
 
     // cost estimation properties
     costs: CostEstimationResidual<'heap, A>,
@@ -231,23 +220,15 @@ struct InlineState<'ctx, 'state, 'env, 'heap, A: Allocator> {
 }
 
 impl<'heap, A: Allocator> InlineState<'_, '_, '_, 'heap, A> {
-    /// Collect all non-recursive callsites for aggressive inlining.
+    /// Collect all callsites for aggressive inlining.
     ///
     /// Used for filter functions which bypass normal heuristics.
-    /// Records inlined SCCs to prevent cycles in subsequent iterations.
-    fn collect_all_callsites(&mut self, body: DefId, mem: &mut InlineStateMemory<A>) {
-        let component = self.components.scc(body);
-
+    /// Self-calls are excluded to prevent panics in `get_disjoint_mut`.
+    fn collect_all_callsites(&self, body: DefId, mem: &mut InlineStateMemory<A>) {
         self.graph
             .apply_callsites(body)
-            .filter(|callsite| self.components.scc(callsite.target) != component)
+            .filter(|callsite| callsite.target != body)
             .collect_into(&mut mem.callsites);
-
-        self.inlined.insert(body, component);
-        for callsite in &mem.callsites {
-            self.inlined
-                .insert(body, self.components.scc(callsite.target));
-        }
     }
 
     /// Collect callsites using heuristic scoring and budget.
@@ -570,7 +551,6 @@ impl<A: BumpAllocator> Inline<A> {
             config: self.config,
             filters,
             loop_breakers,
-            inlined: SparseBitMatrix::new_in(components.node_count(), &self.alloc),
             interner,
             graph,
             costs,
@@ -640,9 +620,6 @@ impl<A: BumpAllocator> Inline<A> {
                 mem.callsites
                     .sort_unstable_by(|lhs, rhs| lhs.kind.cmp(&rhs.kind).reverse());
                 for callsite in mem.callsites.drain(..) {
-                    let target_component = state.components.scc(callsite.target);
-                    state.inlined.insert(filter, target_component);
-
                     state.inline(bodies, callsite);
                 }
 

libs/@local/hashql/mir/src/pass/transform/inline/tests.rs

@@ -1656,3 +1656,69 @@ fn loop_breaker_all_always_directive() {
         "the selected breaker must be one of the SCC members"
     );
 }
+
+/// A filter function that calls into a mutually recursive SCC.
+///
+/// The aggressive phase should inline non-breaker B into the filter, but the
+/// breaker A (visible after B's inlining) must not be expanded. Without the
+/// unconditional breaker check in `FindCallsiteVisitor`, the aggressive phase
+/// would re-expand A on each iteration until the cutoff.
+#[test]
+fn loop_breaker_aggressive_filter_with_recursive_scc() {
+    let heap = Heap::new();
+    let interner = Interner::new(&heap);
+    let env = Environment::new(&heap);
+
+    let a_id = DefId::new(0);
+    let b_id = DefId::new(1);
+    let filter_id = DefId::new(2);
+
+    // A: expensive, calls B (will be selected as breaker)
+    let a = body!(interner, env; fn@a_id/1 -> Int {
+        decl n: Int, cond: Bool, t1: Int, t2: Int, t3: Int, result: Int;
+        bb0() {
+            cond = bin.== n 0;
+            if cond then bb1() else bb2();
+        },
+        bb1() { goto bb3(n); },
+        bb2() {
+            t1 = bin.+ n 1;
+            t2 = bin.+ t1 2;
+            t3 = apply (b_id), t2;
+            goto bb3(t3);
+        },
+        bb3(result) { return result; }
+    });
+
+    // B: cheap, calls A
+    let b = body!(interner, env; fn@b_id/1 -> Int {
+        decl x: Int, result: Int;
+        bb0() {
+            result = apply (a_id), x;
+            return result;
+        }
+    });
+
+    // Filter: calls B
+    let filter = body!(interner, env; [graph::read::filter]@filter_id/1 -> Int {
+        decl x: Int, result: Int;
+        bb0() {
+            result = apply (b_id), x;
+            return result;
+        }
+    });
+
+    let mut bodies = [a, b, filter];
+
+    assert_inline_pass(
+        "loop_breaker_aggressive_filter",
+        &mut bodies,
+        &mut MirContext {
+            heap: &heap,
+            env: &env,
+            interner: &interner,
+            diagnostics: DiagnosticIssues::new(),
+        },
+        InlineConfig::default(),
+    );
+}