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

[JunMa via Phabricator via llvm-commits]

junparser added a comment.

In D79078#2038950 <https://reviews.llvm.org/D79078#2038950>, @lebedev.ri wrote:

> I'll defer to @spatel, although i semi-weakly insist that adding such cut-offs is weird.
>  OTOH if this pass is taught to form such reductions we would have caught this regression for free,
>  because after D79799 <https://reviews.llvm.org/D79799> we would have ended up in an endless combine loop here.
>
> In D79078#2010024 <https://reviews.llvm.org/D79078#2010024>, @spatel wrote:
>
> > I agree that we need a phase ordering test - see llvm/test/Transforms/PhaseOrdering/X86/addsub.ll as an example test file that describes some expected handshake between vector-combine and other passes. Also, please commit the new test with complete baseline CHECK lines (use utils/update_test_checks.py), and then update the file to show the diffs in this review. Let me know if that's not clear.
>
>
> Almost there - it should be next to the example test, not where it is now.
>
> In D79078#2010024 <https://reviews.llvm.org/D79078#2010024>, @spatel wrote:
>
> > I also sympathize with trying to solve this here rather than SLP. One of the reasons vector-combine exists is because SLP became too hard to reason about. In hindsight, we should have created a separate pass for reductions - those are not traditional SLP concerns. Just my opinion. :)
>
>
> I'm not sure what you have in mind here?
>  That *this* pass should also form such reductions?
>  Or that we should not disturb them after SLP formed them?
>  Or something else?
>
> In D79078#2031347 <https://reviews.llvm.org/D79078#2031347>, @junparser wrote:
>
> > hi @lebedev.ri , sorry for the late response.
> >  I have checked the reduction tree build in SLPVectorizer, for now it only support same operation. It seems we can revert the transformation when match reduction operation in matchAssociativeReduction. However, I don't think it is a good idea.
>
>
> I'm having trouble parsing this response.
>  Are you saying that SLP can not be taught about this,
>  that it should not be taught about this or ???.


I'm saying that SLP should not be taught about this. I totally agree with @spatel that splitting reduction transformation off from SLP and recognizing these forms in that pass.


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

https://reviews.llvm.org/D79078