From d588ebbb49824d0503a6565c88e6bd70a4df1ba6 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 25 Sep 2026 02:07:36 +0000 Subject: [PATCH] mask-risc: charge strided predicates' extra passes in op_histogram The strided predicates have no *_under kernel, so the executor runs a gated one as the strided kernel plus mask_and_assign, and NeU32Strided is always the eq kernel plus mask_not_assign. op_histogram sent all three through the generic predicate arm, so mask_passes() under-reported gated Eq/Match by one and Ne by one or two. They are now charged like the gated Range: the gate's and to two_input, Ne's complement to not. Test pins every case, with the ungated Eq/Match silence twin; disable-verified (dropping the arm fails it). Codex P2 on #1284. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DCEP2fZdHYdMCtcEVcTpS2 --- crates/lance-graph-mask-risc/src/ir.rs | 71 ++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/crates/lance-graph-mask-risc/src/ir.rs b/crates/lance-graph-mask-risc/src/ir.rs index fd1773896..a8b652fbe 100644 --- a/crates/lance-graph-mask-risc/src/ir.rs +++ b/crates/lance-graph-mask-risc/src/ir.rs @@ -888,6 +888,34 @@ impl Program { h.two_input += 1; } } + // The strided predicates have no `*_under` kernel either, so a + // gated one is the strided kernel followed by `mask_and_assign` + // — one extra `and`, charged to `two_input` like the gated + // range above. `NeU32Strided` is also `!(==)`: the eq kernel + // and then `mask_not_assign`, a `not` pass on every call, + // gated or not. (Codex P2, PR #1284.) + MaskOp::Pred { + pred: + Pred::EqU32Strided { .. } + | Pred::NeU32Strided { .. } + | Pred::MatchFacetStrided { .. }, + under, + .. + } => { + h.predicates += 1; + if matches!( + op, + MaskOp::Pred { + pred: Pred::NeU32Strided { .. }, + .. + } + ) { + h.not += 1; + } + if under.is_some() { + h.two_input += 1; + } + } MaskOp::Pred { .. } => h.predicates += 1, MaskOp::And { .. } | MaskOp::Or { .. } @@ -1314,4 +1342,47 @@ mod tests { assert_eq!(h.mask_passes(), 3); assert_eq!(p.scratch_slots, 4); } + + /// FAILS IF: the histogram forgets the passes the executor really spends + /// on a strided predicate — the `and` a gate costs (no `*_under` strided + /// kernel exists) and the `not` that turns `NeU32Strided`'s eq kernel into + /// `!=`. Silence twin: an UNGATED Eq or Match is one predicate and nothing + /// else, exactly like a lane predicate. + #[test] + fn op_histogram_charges_the_strided_predicates_extra_passes() { + let pred = |pred, under| { + Program::new( + vec![MaskOp::Pred { + pred, + under, + dst: 0, + }], + Terminal::Count { + mask: Operand::Scratch(0), + }, + ) + .op_histogram() + }; + let gate = Some(Operand::Plane(0)); + let eq = Pred::EqU32Strided { lane: 0, v: 7 }; + let ne = Pred::NeU32Strided { lane: 0, v: 7 }; + let facet = Pred::MatchFacetStrided { + lane: 0, + pattern: [0; 12], + care: [0xFF; 12], + }; + // Silence twin: ungated Eq / Match spend no mask pass. + assert_eq!(pred(eq, None).mask_passes(), 0); + assert_eq!(pred(facet, None).mask_passes(), 0); + // A gate is one `and`. + assert_eq!(pred(eq, gate).two_input, 1); + assert_eq!(pred(facet, gate).mask_passes(), 1); + // `!=` is always one `not`, plus the gate's `and` when gated. + assert_eq!(pred(ne, None).not, 1); + assert_eq!(pred(ne, None).mask_passes(), 1); + assert_eq!(pred(ne, gate).mask_passes(), 2); + for p in [eq, ne, facet] { + assert_eq!(pred(p, gate).predicates, 1, "{p:?} is still one predicate"); + } + } }