halide / Halide

a language for fast, portable data-parallel computation
https://halide-lang.org
Other
5.91k stars 1.07k forks source link

Select condition vector lanes must match the true and false value #8465

Closed abadams closed 2 weeks ago

abadams commented 2 weeks ago

In our Select node, I believe the ability for the condition to be a scalar while the value is a vector was a mistake that just adds complexity. It's easy enough to just check if the condition is a broadcast if you really care about that case.

One place it added a bunch of complexity is that it meant in the RHS of the simplifier rules, sometimes you needed an implicit broadcast. These checks for an implicit broadcast on every binary node construction were responsible for about a third of the code size in Simplify_Sub.o! It's also unclear if it respects the reduction order we use to prove the simplifier terminates, because those rules were turning an implicit broadcast in the IR into a new actual Broadcast node, which may increase the total number of IR nodes, which the simplifier is not supposed to do. Now the Broadcast node is there explicitly in the input.

The select helper in IROperator.h can still take scalar conditions - but it now broadcasts them before calling Select::make. This matches the type-matching behavior of the other helpers in IROperator.h.

This shaves ~600k of code size from the compiler, and speeds up compilation of Simplify_Sub.o (and presumably other large simplifier modules) by 20%.

abadams commented 2 weeks ago

Failures seem to be vulkan related, for which I believe fixes are in progress.