[PATCH] D79078: [VectorCombine] Leave reduction operation to SLP

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In D79078#2040924 <https://reviews.llvm.org/D79078#2040924>, @junparser wrote:

> In D79078#2040191 <https://reviews.llvm.org/D79078#2040191>, @spatel wrote:
>
> > I added slightly modified versions of the tests here with:
> >  rG6211830fbabd <https://reviews.llvm.org/rG6211830fbabd439a41f4c83d3c8ede92019cde3f>
> >  rG43017ceb7841 <https://reviews.llvm.org/rG43017ceb7841c7a0700e5401e79c2f1a28caec5e>
> >
> > Because they are affected by a change that I split off from D79799 <https://reviews.llvm.org/D79799>:
> >  rG81e9ede3a2db <https://reviews.llvm.org/rG81e9ede3a2db32487c15dc20d5d0be6392fb62bc>
> >
> > Please rebase (although given what we've discussed here, I'm not sure how we want to solve the general problem of matching/transforming vector reductions).
>
>
> Yes, this patch just avoid the transforming.  can we handle such form at the end of this pass (revert it to reduction form)?


It raises a question: are we comfortable transforming IR to the (still experimental) reduction intrinsics?
http://llvm.org/docs/LangRef.html#experimental-vector-reduction-intrinsics

I'm finding a related problem with x86 horizontal math ops: -vector-combine does some locally profitable transforms, and SLP can't recognize the larger pattern now.
I think the quick solution will be to move this pass after SLP until we can do bigger changes like recognize reductions here (or possibly in instcombine if we are ready to canonicalize to the intrinsics).


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

https://reviews.llvm.org/D79078