Skip to content

Inject LetStmt of variables that are used downstream modulo some constant. - #9428

Draft
mcourteaux wants to merge 6 commits into
mcourteaux/aligned-split-cleanfrom
mcourteaux/inject-var-mod-letstmt
Draft

Inject LetStmt of variables that are used downstream modulo some constant.#9428
mcourteaux wants to merge 6 commits into
mcourteaux/aligned-split-cleanfrom
mcourteaux/inject-var-mod-letstmt

Conversation

@mcourteaux

Copy link
Copy Markdown
Contributor

Continues on #9409. Makes aligned splits work on an aligned split, for which the inner loop is then vectorized with an additional split factor.

Idea for the lowering pass by @abadams

Breaking changes

Checklist

  • Tests added or updated (not required for docs, CI config, or typo fixes)
  • Documentation updated (if public API changed)
  • Python bindings updated (if public API changed)
  • Benchmarks are included here if the change is intended to affect performance.
  • Commits include AI attribution where applicable (see Code of Conduct)

@mcourteaux
mcourteaux force-pushed the mcourteaux/inject-var-mod-letstmt branch from 89c9ac0 to b3e0958 Compare September 4, 2026 20:10
@codecov

codecov Bot commented Sep 5, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.00000% with 32 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (mcourteaux/aligned-split-clean@bf66758). Learn more about missing BASE report.

Files with missing lines Patch % Lines
src/InjectModuloVars.cpp 83.93% 13 Missing and 18 partials ⚠️
src/Lower.cpp 80.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@                        Coverage Diff                        @@
##             mcourteaux/aligned-split-clean    #9428   +/-   ##
=================================================================
  Coverage                                  ?   70.27%           
=================================================================
  Files                                     ?      262           
  Lines                                     ?    79764           
  Branches                                  ?    19453           
=================================================================
  Hits                                      ?    56052           
  Misses                                    ?    17889           
  Partials                                  ?     5823           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

mcourteaux and others added 6 commits September 8, 2026 13:34
… LetStmt

Distilled from NeonRAW's BilinearDemosaicAlignedGenerator, which selects an
interpolation per Bayer phase with a mux over (x - bayer_offset) % 2 and is
scheduled with splits aligned to that same offset so the phase is constant
per tile. Twelve muxes survive lowering there; this reduces it to two.

The producer is computed inside the consumer's aligned tile and then split
again to vectorize. That second split binds the reconstructed loop variable
to a LetStmt whose value carries the "+ off" from the aligned min. The
simplifier can see through a Let, but not through a LetStmt, so at the use
site it only ever sees "- off" applied to an opaque name, and (x - off) % 2
never reduces to a function of the inner loop variable alone.

Confirmed by simplifying the same expression both ways: behind a LetStmt
neither phase folds, while inlining the value folds both to constants -- and
it still folds if the value is inlined only after the mod-2 sign flip has
already rewritten "- off" into "+ off", so the LetStmt is the sole cause.

The test fails at present; it documents the missed simplification. The
generated code is correct, only the muxes are not resolved.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JbXuRsMMLQDkqE3mwtEKMs
A vectorized loop over a repeating pattern only resolves its phases at
compile time if it gets deinterleaved by the pattern's period, so that each
resulting slice has a single phase. The stride search stopped at four, which
covered a Bayer sensor's period of two but not an X-Trans sensor's six.

It can't simply go to eight: deinterleaving a period of eight into eight
slices displaces permutes that a target may do in one instruction, and three
of simd_op_check_hvx's vdelta checks regress that way. Seven is the largest
value that leaves those alone. A period of eight still resolves if the loop
is unrolled rather than vectorized.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JbXuRsMMLQDkqE3mwtEKMs
Generalizes the test over the period of the pattern the mux selects on, and
runs it for two (Bayer), three, and six (X-Trans) rather than two alone.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JbXuRsMMLQDkqE3mwtEKMs
A value is often needed only modulo some constant: x % 5, (x + 3) % 5 and
(x + 2*y) % 3 all need no more of x than x % 5. Reducing the value where it
is defined frequently collapses it. A loop variable reconstructed by an
aligned split looks like xo * 16 + offset, which modulo two is just offset.

The simplifier can't discover that at the use site. A LetStmt hides the
value behind a name, and it won't substitute a whole expression back in. It
will substitute a variable, so this pass binds the reduced value to one:
(x - offset) % 2 becomes (x.mod.2 - offset) % 2, and with x.mod.2 bound to
offset that folds to zero. This is what lets a demosaicer scheduled with
aligned splits resolve its Bayer phases at compile time; twelve muxes
survived lowering in the distilled test before.

Only reductions that collapse to a constant or a single variable are bound,
so the pass can never grow the IR, and the re-simplification afterwards is
skipped when it bound nothing. On a pipeline that benefits, compiling gets
faster rather than slower, because everything downstream sees smaller IR:
0.83s against 0.96s for twenty compilations of a demosaicer. On one that
doesn't, it is free: 0.354s against 0.349s for a plain blur.

Reducing a value looks through the lets it is built from, to a bounded
depth, so that terms which cancel get the chance to meet, and it takes the
conditions of dominating asserts as assumptions -- a require() on an offset
is often the only thing saying it is small enough to be its own remainder.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016WxFGPpnekN4bH6sMCsQBQ
The correctness test needs no entry: the Makefile globs test/correctness.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016WxFGPpnekN4bH6sMCsQBQ
Co-authored-by: Claude <noreply@anthropic.com>
@alexreinking
alexreinking force-pushed the mcourteaux/inject-var-mod-letstmt branch from bf39e80 to be807c1 Compare September 8, 2026 17:34
@alexreinking

Copy link
Copy Markdown
Member

Andrew and I are mulling over the degree to which the simplifier should own the mechanism of simplifying expressions with ring operations modulo constants. That's as distinct from the policy of when to branch a let for which it might be profitable.

@mcourteaux

Copy link
Copy Markdown
Contributor Author

Dev Meeting:

  • A new simplifier rewrite rule does the trick in my particular case:
       rewrite(x * c0 + (y * c1 + z), (x * fold(c0 / c1) + y) * c1 + z, c0 % c1 == 0) ||
    This alleviates my immediate need for this PR.
  • The modulo reduction logic in this PR should probably be unified with the simplifier itself. A ScopedValue can hold the modulo-constant context for which we are currently simplifying. We need to be careful to clear it at the right places before recursing.
  • The root cause of this being an issue at all is that the simplifier does not inline let-bound variables. Instead it does peeling of operations with constant operands into the use-site. This is in my particular case insufficient. The simplifier not inlining lets is something that comes up every now and then as a limitation. We discussed the possibility of an intrinsic in the Halide IR that means: inline lets in the contained expression. Or alternatively an intrinsic saying "this Expr must eventually simplify to a constant". Such an intrinsic now gives control to the author of the pipeline. A typical use case would be: mux(must_simplify_to_const(my_expr), ...). One thing to be wary about here is that if loop partitioning ever supports scalar tails, a mux like this might not simplify anymore due when the steady state was unrolled.
  • Andrew briefly thought of something like "reverse peeling". You may try to wrap the value of the variable used further into same operation-tree as where the variable is used, and see if anything simplifies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants