[llvm] [InstCombine] Fold binary op of reductions. (PR #121567)

David Green via llvm-commits llvm-commits at lists.llvm.org
Sun Jan 12 03:12:00 PST 2025


davemgreen wrote:

> Before this combine worked for Sub in inst combine - was this an oversight?

Yeah maybe. In my experience adds are much more common than the other reductions, and sub(reduce.add, reduce.add) would be less common than plain adds. Maybe it wasn't done for the more obvious `add` case because it can cause problems.
The reduction thing is more obvious when you look at the flags needed to make the fp version valid.

> As I said before, vertical vector ops are always cheaper than horizontal ones

I believe that is mostly true, but some targets can have relatively efficient reductions. It looks like MVE will already convert back if needed https://godbolt.org/z/6YGqde37s.
I think the larger problem is extending reductions where the pattern can be broken by the reassociation. https://godbolt.org/z/cohazhd4x. Especially for mla variants like the dotproduct instructions https://godbolt.org/z/K1E3WqrcP.

> If not, we can move it into VectorCombine and use InstructionCost to see whether the transformation is a win.

We might not have costs (that don't go via getExtendedReductionCost / getMulAccReductionCost), as multiple-instructions patterns can be awkward to cost model well and it might not have come up before.

> Perhaps we could have both InstCombine and DAGCombiner transformations guarded by different target hooks?

Instcombine, for better or worse, is considered a canonicalization pass that isn't controlled by target hooks. Vector combine is the place cost-modelled combines for vector is usually done. We could also consider this to be the canonical pattern and undo the transform in the backend if needed and we could do so reliably.

https://github.com/llvm/llvm-project/pull/121567


More information about the llvm-commits mailing list