halide / Halide

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

Debate: Should `never_partition_all()` should be the default? #8347

Open mcourteaux opened 3 months ago

mcourteaux commented 3 months ago

I find myself literring my code with this directive everywhere. Now that we have it, and we all were able to use it for a while, I am tempted to argue that it should be the default. Loop partitioning is quite an extensive transformation that ...

Overall, I think that loop partitioning can be beneficial, but should be a conscious choice of the developer of the pipeline. My personal preference would go to loop partitioning being disabled by default; but as an alternative, I can see a lot of benefit into making the Auto strategy a lot smarter into deciding when to, and when not (which would change it from actually being EverywherePossible).

Opinions? Perhaps something for the dev-meeting?

steven-johnson commented 3 months ago

Was this discussed, and if so, were there any conclusions?

mcourteaux commented 3 months ago

I unfortunately could be there during the last dev-meeting. I don't know if it was discussed yet.

abadams commented 3 months ago

We can discuss it today, but it would tank the performance of a whole lot of existing production code by turning vector loads into vector gathers.

abadams commented 3 months ago

At the dev meeting we discussed making a target flag that changes the default

mcourteaux commented 3 months ago

it would tank the performance of a whole lot of existing production code by turning vector loads into vector gathers.

That would potentially be a good heuristic for the Auto strategy: does it turn load/stores into gathers/scatters. A simple TailStrategy::ShiftInwards would not suffer from loads/stores being turned into gather/scatters.