[PATCH] D143786: [X86] Add `TuningPreferShiftShuffle` for when Shifts are preferable to shuffles.
Jordan Rupprecht via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 1 17:03:47 PST 2023
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
More information about the llvm-commits
mailing list