Add 'branch(cond, a, b)' to have a materialized branch#9194
Add 'branch(cond, a, b)' to have a materialized branch#9194soufianekhiat wants to merge 2 commits into
Conversation
|
Pervasive throughout the compiler is the implicit assumption that within a single expr there are no sequence points or side-effects. It's more than just CSE that this is likely to break. The current way to get a branch from a select is to make the true/false values of the select calls to a Func, and then schedule those Funcs somewhere such that the condition is a constant for the lifetime of the func. This will give you an actual branch around the computation of the Func. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9194 +/- ##
=======================================
Coverage ? 69.40%
=======================================
Files ? 254
Lines ? 78338
Branches ? 18749
=======================================
Hits ? 54370
Misses ? 18460
Partials ? 5508 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
I think exposing a branched variant of Select to users is fine, but I have doubts:
|
About @abadams point: you are right, the no side effect assumption is broad and i I'll try to do something to have it on GPU too. |
|
I have reservations about the tractability of this proposal, too. This fundamentally changes the contract on expressions. I think that, at a minimum, you'd need to test the hypothetical of "what breaks if every I'd say my biggest semantic concern is that bounds inference and multi-stage funcs each restrict the usefulness of this feature. For example: Only the loads from A lesser issue with the above: at a glance, the code looks like a C-like ternary ( I'm curious: what is the shape of the computation you're trying to branch? Maybe there are smaller features we could add that would make it easier to express. I could see adding predicates to update stages without a dummy RDom, or maybe even to pure definitions (I haven't thought all those consequences through). |
|
@soufianekhiat — would it be sufficient to allow branching only as the outermost "expression"? In that case, it could become a part of the Stage's definition and |
|
Let be sure I understand. The ideas f(x) = g(x) + branch(cond, a, b) * 2; // branch buried in the middle -> contract problemTo: And for the alternative: f(x) = undef<int>(); // no default work
RDom ra(0, N); ra.where(cond(ra));
f(ra) = expensive_a(ra); // -> for r: if(cond) f[r] = expensive_a(r) (a only runs where cond)
RDom rb(0, N); rb.where(!cond(rb));
f(rb) = expensive_b(rb); // -> for r: if(!cond) f[r] = expensive_b(r)I don't like the RDom make everything two stage vs branch which had simpler "expressiveness". Solution:
|
Context
Today when we write
select(cond, a, b)in a Halide kernel, both sideaand
bare always evaluated, and then we pick one. This is fine most of thetime but when one side is really expensive it is a waste, we compute things
for nothing.
The compiler already have internally what we need for this: the
if_then_elseintrinsic. Unlike
Select, it "promise to evaluate exactly one of theconditions" and it already lower to a real control flow branch (basic blocks +
conditional branch + phi in LLVM, real if/else in the C backend, blocks + phi
in Vulkan). The only thing missing was a way for the user to reach it.
What this PR do
Add a new method
Expr::branch(). You call it on the result of aselectandit give you back the same thing but as an explicit branch that evaluate only
the taken side:
f(x) = select(cond, cheap, very_expensive(x)).branch();It is just a thin rewrite from the Select node to the if_then_else
intrinsic, so all the existing machinery (lowering, simplification, bounds,
cost model, serialization, every backend) already handle it.
It also work with the multi way select, the full chain become a full
if / else-if / else chain of branches:
CSE barrier
There was one real problem. The CSE pass don't know that the two arms of
if_then_else are lazy. So a subexpression used only inside one arm could be
lifted into a let above the branch and then be evaluated all the time, which
kill the whole point (and could even run an operation the branch was there to
protect).
So this PR also teach ComputeUseCounts in CSE.cpp to not count the uses
inside the value arms of if_then_else. The condition is still counted
normally because it is always evaluated. Like this an arm only subexpression is
never hoisted out of the branch. This stay conservative for the two callers
that use lift_all, it only make them a bit more careful for the arms. The
within arm duplicates are anyway cleaned later by LLVM.
Limitations (documented in the header)
context .branch() degrade back to a select (both sides evaluated). This is
expected, not a bug.
arm of a select. A select buried inside some arithmetic in an arm stay a
normal data select.
Tests
New test test/correctness/branch.cpp (added to the correctness group):
the type
the branch and is not hoisted out (checked on the lowered IR)
I also run mux, multi_way_select, likely and tuple_select to check the CSE
change do not break the select / if_then_else lowering, they all pass.
Files
Disclosure: Co-author Claude Opus 4.8