[PATCH] D141870: [DAG] Fold fadd(vecreduce(a), vecreduce(b)) into vecreduce(fadd(a,b))

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

In D141870#4057159 <https://reviews.llvm.org/D141870#4057159>, @RKSimon wrote:

> Should we be doing this at IR level as well? https://simd.godbolt.org/z/aTE8qjMET (to be clear - I'm not expecting this patch to address it)

The case that was reported to me was a <16 x float> reduction and a <4 x float> reduction added together, which needs to be handled after legalization. Similar to fadd_reduct_reassoc_v4v8f32 but with a 16x and more going on.

I'm not sure that certain architectures would like the combine in for all reductions. It can depend on whether the reduction+add is cheap. MVE might prefer multiple reductions for integer adds for example, which can require quite precise cost modelling, unfortunately.

In D141870#4057153 <https://reviews.llvm.org/D141870#4057153>, @spatel wrote:

> We probably want this for all associate reductions - fmul, min/max, integer ops, etc. Generalize into a helper function that takes an opcode and maps it to the corresponding reduction opcode? Subsequent patches then just need to add set of tests and another case into a switch or something like that.

Sounds good. I can take a look, and see if it needs a target-hook to prevent the transform for any backends.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141870/new/

https://reviews.llvm.org/D141870