[PATCH] D53037: [InstCombine] combine a shuffle and an extract subvector shuffle

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In D53037#1385727 <https://reviews.llvm.org/D53037#1385727>, @efriedma wrote:

> That case is specifically interesting because when it's split, each half is lowered to a different shuffle, and both shuffles are expensive.
>
> Obviously you could try to find that specific pattern in SelectionDAG on AArch64, but I'm not sure it makes sense to expect the backend to try; instcombine probably shouldn't be creating difficult-to-solve optimization puzzles.


The goal for this patch was to make things easier for the backend, but I didn't think of this case of course. How about limiting the transform like this:

  Index: InstCombine/InstCombineVectorOps.cpp
  ===================================================================
  --- InstCombine/InstCombineVectorOps.cpp	(revision 353169)
  +++ InstCombine/InstCombineVectorOps.cpp	(working copy)
  @@ -1498,6 +1498,11 @@
     if (!match(Op0, m_ShuffleVector(m_Value(X), m_Value(Y), m_Constant(Mask))))
       return nullptr;
   
  +  // Be extra conservative with shuffle transforms. If we can't kill the 1st
  +  // shuffle, then combining may result in worse codegen.
  +  if (!Op0->hasOneUse())
  +    return nullptr;
  +
     // We are extracting a subvector from a shuffle. Remove excess elements from
     // the 1st shuffle mask to eliminate the extract.
     //


Repository:
  rL LLVM

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

https://reviews.llvm.org/D53037