[PATCH] D143786: [X86] Add `TuningPreferShiftShuffle` for when Shifts are preferable to shuffles.

[Jordan Rupprecht via Phabricator via llvm-commits]

rupprecht added a comment.

> I think the issue is:
>
>   +      // Byte shifts can be slower so only match them on second attempt.
>   +      if (Order == 0 &&
>   +          (Shuffle == X86ISD::VSHLDQ || Shuffle == X86ISD::VSRLDQ))
>   +        continue;
>
> It comes before the check of
>
>   +      if (0 < ShiftAmt && (!ShuffleVT.is512BitVector() || Subtarget.hasBWI() ||
>   +                           32 <= ShuffleVT.getScalarSizeInBits())) {
>   +        PermuteImm = (unsigned)ShiftAmt;
>   +        return true;
>   +      }
>
> and the `0 < ShiftAmt` check if basically a check if actually found/set `Shuffle`.
> Don't think the bug actually can change behavior but is bug none the less.
>
> Will post patch to fix.

Yeah, that looks like the cause. There's another place where matchShuffleAsShift is called that seems problematic. Adding some assertions can catch the issue outside of an msan build:

  $ git diff
  diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
  index 71dad73cfd9b..6af0ce2fd14b 100644
  --- a/llvm/lib/Target/X86/X86ISelLowering.cpp
  +++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
  @@ -14118,6 +14118,7 @@ static SDValue lowerShuffleAsShift(const SDLoc &DL, MVT VT, SDValue V1,
       V = V2;
     }
  
  +  assert(ShiftAmt >= 0 && "matchShuffleAsShift failed twice");
     if (BitwiseOnly && (Opcode == X86ISD::VSHLDQ || Opcode == X86ISD::VSRLDQ))
       return SDValue();
  
  @@ -38829,6 +38830,7 @@ static bool matchUnaryPermuteShuffle(MVT MaskVT, ArrayRef<int> Mask,
         int ShiftAmt =
             matchShuffleAsShift(ShuffleVT, Shuffle, MaskScalarSizeInBits, Mask, 0,
                                 Zeroable, Subtarget);
  +      assert(ShiftAmt >= 0 && "matchShuffleAsShift failed");
         // Byte shifts can be slower so only match them on second attempt.
         if (Order == 0 &&
             (Shuffle == X86ISD::VSHLDQ || Shuffle == X86ISD::VSRLDQ))



================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:14121
 
+  if (BitwiseOnly && (Opcode == X86ISD::VSHLDQ || Opcode == X86ISD::VSRLDQ))
+    return SDValue();
----------------
here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143786