Skip to content

Commit fb80624

Browse files
committed
chore: follow projections on param edges
1 parent 0012314 commit fb80624

6 files changed

Lines changed: 263 additions & 4 deletions

File tree

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

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,20 @@ fn traverse<'heap, A: Allocator + Clone>(
188188
/// constants). This function checks whether all non-cyclic predecessors, from both
189189
/// sources, resolve to the same value.
190190
///
191+
/// # Projection-aware consensus
192+
///
193+
/// When the queried place has a projection suffix (e.g., resolving `x.0` where `x` is a
194+
/// block parameter), consensus is checked on the *fully resolved* result per predecessor,
195+
/// not on the partially resolved predecessor bases. This is necessary because different
196+
/// predecessor locals can still agree on a projected field.
197+
///
198+
/// For example, if predecessor A passes `(42, u)` and predecessor B passes `(42, v)`,
199+
/// the bases disagree but `A.0 == B.0 == 42`. The algorithm resolves each predecessor
200+
/// through the full projection suffix before comparing, so this case correctly yields
201+
/// `Resolved(42)` rather than `Incomplete(x.0)`.
202+
///
203+
/// # Cycle handling
204+
///
191205
/// Cyclic predecessors ([`Backtrack`]) are filtered out before consensus checking.
192206
/// Since [`Param`] edges are identity transfers, the value is fully determined by
193207
/// the non-cyclic init edges. If only cyclic predecessors exist (no external source),
@@ -236,12 +250,35 @@ fn resolve_params<'heap, A: Allocator + Clone>(
236250
};
237251

238252
// Resolve all predecessor candidates and check consensus.
253+
//
254+
// When the queried place has projections (e.g., `x.field`), each predecessor is resolved
255+
// through the full projection suffix before consensus comparison. If `traverse` returns
256+
// `Continue(local)` (predecessor base resolved to a bare local), we call `resolve` on
257+
// `local.projections` to complete the resolution. This ensures consensus is checked on
258+
// the final value, not intermediate bases that may differ structurally but agree on the
259+
// projected component.
260+
//
239261
// Cyclic predecessors (Backtrack) are skipped: since Param edges are identity transfers,
240262
// the value is fully determined by the non-cyclic init edges. If only cyclic predecessors
241263
// exist, we cannot resolve (the value has no external source).
242-
let graph_edges = graph
243-
.outgoing_edges(place.local)
244-
.map(|edge| traverse(rec_state.cloned(), place, edge));
264+
let graph_edges = graph.outgoing_edges(place.local).map(|edge| {
265+
let result = traverse(rec_state.cloned(), place, edge);
266+
267+
match result {
268+
// Predecessor resolved to a bare local, but the query has remaining projections.
269+
// Finish resolving through the projection suffix so consensus compares final values.
270+
ControlFlow::Continue(local) if !place.projections.is_empty() => {
271+
ControlFlow::Break(resolve(
272+
rec_state.cloned(),
273+
PlaceRef {
274+
local,
275+
projections: place.projections,
276+
},
277+
))
278+
}
279+
ControlFlow::Continue(_) | ControlFlow::Break(_) => result,
280+
}
281+
});
245282
let constant_edges = graph
246283
.constant_bindings
247284
.iter_by_kind(place.local, EdgeKind::Param)
@@ -314,7 +351,11 @@ fn resolve_params<'heap, A: Allocator + Clone>(
314351
/// the data ultimately originates. The algorithm handles three types of edges:
315352
///
316353
/// - **[`Load`]**: Always followed transitively (a load has exactly one source)
317-
/// - **[`Param`]**: Followed only if all predecessors agree on the same source (consensus)
354+
/// - **[`Param`]**: Followed only if all predecessors agree on the same source (consensus).
355+
/// Consensus is checked on fully resolved results: when the queried place has projections, each
356+
/// predecessor is resolved through the complete projection suffix before comparison. This allows
357+
/// resolution through φ-nodes where predecessor bases differ but the projected component agrees
358+
/// (e.g., `(42, a)` and `(42, b)` agree on field `.0`).
318359
/// - **[`Index`]/[`Field`]**: Matched against projections to trace through aggregates
319360
///
320361
/// Resolution terminates with:

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

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -718,3 +718,151 @@ fn load_param_mixed() {
718718
},
719719
);
720720
}
721+
722+
/// Tests that Param consensus resolves through projections when predecessors are
723+
/// different tuples but the queried field is the same constant.
724+
///
725+
/// Both paths construct different tuples (`a = (42, u)`, `b = (42, v)`) but the
726+
/// `.0` field is the same constant `42` in both. Current algorithm compares the
727+
/// tuple bases (`a` vs `b`), which disagree, so it returns `Incomplete(x.0)`.
728+
/// Correct behavior: resolve `a.0` and `b.0` individually, find they both yield
729+
/// `42`, and return `Resolved(42)`.
730+
#[test]
731+
fn param_consensus_projected_field_const() {
732+
let heap = Heap::new();
733+
let interner = Interner::new(&heap);
734+
let env = Environment::new(&heap);
735+
736+
let body = body!(interner, env; fn@0/0 -> Int {
737+
decl u: Int, v: Int, a: (Int, Int), b: (Int, Int), cond: Int, x: (Int, Int), result: Int;
738+
@proj x_0 = x.0: Int;
739+
740+
bb0() {
741+
u = input.load! "u";
742+
v = input.load! "v";
743+
cond = input.load! "cond";
744+
a = tuple 42, u;
745+
b = tuple 42, v;
746+
if cond then bb1() else bb2();
747+
},
748+
bb1() {
749+
goto bb3(a);
750+
},
751+
bb2() {
752+
goto bb3(b);
753+
},
754+
bb3(x) {
755+
result = load x_0;
756+
return result;
757+
}
758+
});
759+
760+
assert_data_dependency(
761+
"param_consensus_projected_field_const",
762+
&body,
763+
&mut MirContext {
764+
heap: &heap,
765+
env: &env,
766+
interner: &interner,
767+
diagnostics: DiagnosticIssues::new(),
768+
},
769+
);
770+
}
771+
772+
/// Tests that Param consensus resolves through projections when predecessors are
773+
/// different tuples but the queried field is the same place.
774+
///
775+
/// Both paths construct different tuples (`a = (src, u)`, `b = (src, v)`) but the
776+
/// `.0` field is the same local `src` in both. Current algorithm compares the
777+
/// tuple bases (`a` vs `b`), which disagree, so it returns `Incomplete(x.0)`.
778+
/// Correct behavior: resolve `a.0` and `b.0` individually, find they both yield
779+
/// `src`, and return `Resolved(src)`.
780+
#[test]
781+
fn param_consensus_projected_field_place() {
782+
let heap = Heap::new();
783+
let interner = Interner::new(&heap);
784+
let env = Environment::new(&heap);
785+
786+
let body = body!(interner, env; fn@0/0 -> Int {
787+
decl src: Int, u: Int, v: Int, a: (Int, Int), b: (Int, Int), cond: Int, x: (Int, Int), result: Int;
788+
@proj x_0 = x.0: Int;
789+
790+
bb0() {
791+
src = input.load! "src";
792+
u = input.load! "u";
793+
v = input.load! "v";
794+
cond = input.load! "cond";
795+
a = tuple src, u;
796+
b = tuple src, v;
797+
if cond then bb1() else bb2();
798+
},
799+
bb1() {
800+
goto bb3(a);
801+
},
802+
bb2() {
803+
goto bb3(b);
804+
},
805+
bb3(x) {
806+
result = load x_0;
807+
return result;
808+
}
809+
});
810+
811+
assert_data_dependency(
812+
"param_consensus_projected_field_place",
813+
&body,
814+
&mut MirContext {
815+
heap: &heap,
816+
env: &env,
817+
interner: &interner,
818+
diagnostics: DiagnosticIssues::new(),
819+
},
820+
);
821+
}
822+
823+
/// Tests that a cycle with a loop-invariant projected field resolves correctly.
824+
///
825+
/// The cycle is `x -> x` via the back-edge in `bb1`. The init edge provides
826+
/// `init = (src, other)`. The back-edge reconstructs `t = (x.0, other)`,
827+
/// preserving `x.0` across iterations. So `x.0` is loop-invariant and should
828+
/// resolve to `src`. Current algorithm compares `init` vs `t` as bases, which
829+
/// disagree, yielding `Incomplete(x.0)`. Correct behavior: resolve the full
830+
/// `init.0 = src` and see that `t.0 = x.0` is a cyclic identity, so the
831+
/// non-cyclic init determines the answer.
832+
#[test]
833+
fn param_cycle_invariant_projected_field() {
834+
let heap = Heap::new();
835+
let interner = Interner::new(&heap);
836+
let env = Environment::new(&heap);
837+
838+
let body = body!(interner, env; fn@0/0 -> Int {
839+
decl src: Int, other: Int, init: (Int, Int), x: (Int, Int), t: (Int, Int), cond: Int, result: Int;
840+
@proj x_0 = x.0: Int;
841+
842+
bb0() {
843+
src = input.load! "src";
844+
other = input.load! "other";
845+
cond = input.load! "cond";
846+
init = tuple src, other;
847+
goto bb1(init);
848+
},
849+
bb1(x) {
850+
t = tuple x_0, other;
851+
if cond then bb1(t) else bb2(x_0);
852+
},
853+
bb2(result) {
854+
return result;
855+
}
856+
});
857+
858+
assert_data_dependency(
859+
"param_cycle_invariant_projected_field",
860+
&body,
861+
&mut MirContext {
862+
heap: &heap,
863+
env: &env,
864+
interner: &interner,
865+
diagnostics: DiagnosticIssues::new(),
866+
},
867+
);
868+
}

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,8 @@ impl<'heap, A: Allocator> InstSimplifyVisitor<'_, 'heap, A> {
309309
(BinOp::BitAnd, 0) if is_bool => {
310310
Some(RValue::Load(Operand::Constant(Constant::Int(false.into()))))
311311
}
312+
// 0 & rhs => 0 (annihilator)
313+
(BinOp::BitAnd, 0) => Some(RValue::Load(Operand::Constant(Constant::Int(0.into())))),
312314
(BinOp::BitAnd, _) => None,
313315
// 0 | rhs => rhs (identity)
314316
(BinOp::BitOr, 0) => Some(RValue::Load(Operand::Place(rhs))),
@@ -369,6 +371,8 @@ impl<'heap, A: Allocator> InstSimplifyVisitor<'_, 'heap, A> {
369371
(BinOp::BitAnd, 0) if is_bool => {
370372
Some(RValue::Load(Operand::Constant(Constant::Int(false.into()))))
371373
}
374+
// 0 & lhs => 0 (annihilator)
375+
(BinOp::BitAnd, 0) => Some(RValue::Load(Operand::Constant(Constant::Int(0.into())))),
372376
(BinOp::BitAnd, _) => None,
373377
// lhs | 0 => lhs (identity)
374378
(BinOp::BitOr, 0) => Some(RValue::Load(Operand::Place(lhs))),

libs/@local/hashql/mir/tests/ui/pass/data-dependency/param_consensus_projected_field_const.snap

Lines changed: 22 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

libs/@local/hashql/mir/tests/ui/pass/data-dependency/param_consensus_projected_field_place.snap

Lines changed: 22 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

libs/@local/hashql/mir/tests/ui/pass/data-dependency/param_cycle_invariant_projected_field.snap

Lines changed: 22 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)