[llvm] [RISCV] Remove now redundant isElementRotate shuffle lowering [NFCI] (PR #128064)
Philip Reames via llvm-commits
llvm-commits at lists.llvm.org
Thu Feb 20 12:56:16 PST 2025
https://github.com/preames created https://github.com/llvm/llvm-project/pull/128064
This is the first cleanup following the introduction of the new isMaskedSlidePair lowering in 43f2968. As mentioned in that review, the new code is a generalization of the existing isElementRotate, but was staged to make diffs manageable.
This change removes isElementRotate. The tricky bit is making sure that a) the same shuffles hit the same lowering paths (in particular, two element shuffles can match multiple ways) and 2) avoiding DAG canonicalization changes.
>From 33f8d3b9a10194f14a2e4028d2f28b94d77b2540 Mon Sep 17 00:00:00 2001
From: Philip Reames <preames at rivosinc.com>
Date: Thu, 20 Feb 2025 09:35:05 -0800
Subject: [PATCH] [RISCV] Remove now redundant isElementRotate shuffle lowering
[NFCI]
This is the first cleanup following the introduction of the new
isMaskedSlidePair lowering in 43f2968. As mentioned in that review,
the new code is a generalization of the existing isElementRotate, but
was staged to make diffs manageable.
This change removes isElementRotate. The tricky bit is making sure
that a) the same shuffles hit the same lowering paths (in particular,
two element shuffles can match multiple ways) and 2) avoiding DAG
canonicalization changes.
---
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | 144 ++++----------------
1 file changed, 27 insertions(+), 117 deletions(-)
diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
index 18f92faa43c5e..6076fe56416ad 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -4589,10 +4589,6 @@ static bool isMaskedSlidePair(ArrayRef<int> Mask,
return false;
}
- // Avoid matching unconditional slides for now. This is reasonably
- // covered by existing matchers.
- if (SrcInfo[0].first == -1 || SrcInfo[1].first == -1)
- return false;
// Avoid matching vselect idioms
if (SrcInfo[0].second == 0 && SrcInfo[1].second == 0)
return false;
@@ -4601,80 +4597,17 @@ static bool isMaskedSlidePair(ArrayRef<int> Mask,
if ((SrcInfo[0].second > 0 && SrcInfo[1].second < 0) ||
SrcInfo[1].second == 0)
std::swap(SrcInfo[0], SrcInfo[1]);
+ assert(SrcInfo[0].first != -1 && "Must find one slide");
return true;
}
-/// Match shuffles that concatenate two vectors, rotate the concatenation,
-/// and then extract the original number of elements from the rotated result.
-/// This is equivalent to vector.splice or X86's PALIGNR instruction. The
-/// returned rotation amount is for a rotate right, where elements move from
-/// higher elements to lower elements. \p LoSrc indicates the first source
-/// vector of the rotate or -1 for undef. \p HiSrc indicates the second vector
-/// of the rotate or -1 for undef. At least one of \p LoSrc and \p HiSrc will be
-/// 0 or 1 if a rotation is found.
-///
-/// NOTE: We talk about rotate to the right which matches how bit shift and
-/// rotate instructions are described where LSBs are on the right, but LLVM IR
-/// and the table below write vectors with the lowest elements on the left.
-static int isElementRotate(int &LoSrc, int &HiSrc, ArrayRef<int> Mask) {
- int Size = Mask.size();
-
- // We need to detect various ways of spelling a rotation:
- // [11, 12, 13, 14, 15, 0, 1, 2]
- // [-1, 12, 13, 14, -1, -1, 1, -1]
- // [-1, -1, -1, -1, -1, -1, 1, 2]
- // [ 3, 4, 5, 6, 7, 8, 9, 10]
- // [-1, 4, 5, 6, -1, -1, 9, -1]
- // [-1, 4, 5, 6, -1, -1, -1, -1]
- int Rotation = 0;
- LoSrc = -1;
- HiSrc = -1;
- for (int i = 0; i != Size; ++i) {
- int M = Mask[i];
- if (M < 0)
- continue;
-
- // Determine where a rotate vector would have started.
- int StartIdx = i - (M % Size);
- // The identity rotation isn't interesting, stop.
- if (StartIdx == 0)
- return -1;
-
- // If we found the tail of a vector the rotation must be the missing
- // front. If we found the head of a vector, it must be how much of the
- // head.
- int CandidateRotation = StartIdx < 0 ? -StartIdx : Size - StartIdx;
-
- if (Rotation == 0)
- Rotation = CandidateRotation;
- else if (Rotation != CandidateRotation)
- // The rotations don't match, so we can't match this mask.
- return -1;
-
- // Compute which value this mask is pointing at.
- int MaskSrc = M < Size ? 0 : 1;
-
- // Compute which of the two target values this index should be assigned to.
- // This reflects whether the high elements are remaining or the low elements
- // are remaining.
- int &TargetSrc = StartIdx < 0 ? HiSrc : LoSrc;
-
- // Either set up this value if we've not encountered it before, or check
- // that it remains consistent.
- if (TargetSrc < 0)
- TargetSrc = MaskSrc;
- else if (TargetSrc != MaskSrc)
- // This may be a rotation, but it pulls from the inputs in some
- // unsupported interleaving.
- return -1;
- }
-
- // Check that we successfully analyzed the mask, and normalize the results.
- assert(Rotation != 0 && "Failed to locate a viable rotation!");
- assert((LoSrc >= 0 || HiSrc >= 0) &&
- "Failed to find a rotated input vector!");
-
- return Rotation;
+// Exactly matches the semantics of a previously existing custom matcher
+// to allow migration to new matcher without changing output.
+static bool isElementRotate(std::pair<int, int> SrcInfo[2], unsigned NumElts) {
+ if (SrcInfo[1].first == -1)
+ return true;
+ return SrcInfo[0].second < 0 && SrcInfo[1].second > 0 &&
+ SrcInfo[1].second - SrcInfo[0].second == (int)NumElts;
}
// Lower a deinterleave shuffle to SRL and TRUNC. Factor must be
@@ -5585,41 +5518,8 @@ static SDValue lowerVECTOR_SHUFFLE(SDValue Op, SelectionDAG &DAG,
if (SDValue V = lowerVECTOR_SHUFFLEAsRotate(SVN, DAG, Subtarget))
return V;
- // Lower rotations to a SLIDEDOWN and a SLIDEUP. One of the source vectors may
- // be undef which can be handled with a single SLIDEDOWN/UP.
- int LoSrc, HiSrc;
- int Rotation = isElementRotate(LoSrc, HiSrc, Mask);
- if (Rotation > 0) {
- SDValue LoV, HiV;
- if (LoSrc >= 0) {
- LoV = LoSrc == 0 ? V1 : V2;
- LoV = convertToScalableVector(ContainerVT, LoV, DAG, Subtarget);
- }
- if (HiSrc >= 0) {
- HiV = HiSrc == 0 ? V1 : V2;
- HiV = convertToScalableVector(ContainerVT, HiV, DAG, Subtarget);
- }
-
- // We found a rotation. We need to slide HiV down by Rotation. Then we need
- // to slide LoV up by (NumElts - Rotation).
- unsigned InvRotate = NumElts - Rotation;
-
- SDValue Res = DAG.getUNDEF(ContainerVT);
- if (HiV) {
- // Even though we could use a smaller VL, don't to avoid a vsetivli
- // toggle.
- Res = getVSlidedown(DAG, Subtarget, DL, ContainerVT, Res, HiV,
- DAG.getConstant(Rotation, DL, XLenVT), TrueMask, VL);
- }
- if (LoV)
- Res = getVSlideup(DAG, Subtarget, DL, ContainerVT, Res, LoV,
- DAG.getConstant(InvRotate, DL, XLenVT), TrueMask, VL,
- RISCVVType::TAIL_AGNOSTIC);
-
- return convertFromScalableVector(VT, Res, DAG, Subtarget);
- }
-
- if (ShuffleVectorInst::isReverseMask(Mask, NumElts) && V2.isUndef())
+ if (ShuffleVectorInst::isReverseMask(Mask, NumElts) && V2.isUndef() &&
+ NumElts != 2)
return DAG.getNode(ISD::VECTOR_REVERSE, DL, VT, V1);
// If this is a deinterleave(2,4,8) and we can widen the vector, then we can
@@ -5662,7 +5562,9 @@ static SDValue lowerVECTOR_SHUFFLE(SDValue Op, SelectionDAG &DAG,
// Detect an interleave shuffle and lower to
// (vmaccu.vx (vwaddu.vx lohalf(V1), lohalf(V2)), lohalf(V2), (2^eltbits - 1))
int EvenSrc, OddSrc;
- if (isInterleaveShuffle(Mask, VT, EvenSrc, OddSrc, Subtarget)) {
+ if (isInterleaveShuffle(Mask, VT, EvenSrc, OddSrc, Subtarget) &&
+ !(NumElts == 2 &&
+ ShuffleVectorInst::isSingleSourceMask(Mask, Mask.size()))) {
// Extract the halves of the vectors.
MVT HalfVT = VT.getHalfNumVectorElementsVT();
@@ -5698,15 +5600,16 @@ static SDValue lowerVECTOR_SHUFFLE(SDValue Op, SelectionDAG &DAG,
// Recognize a pattern which can handled via a pair of vslideup/vslidedown
// instructions (in any combination) with masking on the second instruction.
- // Avoid matching bit rotates as slide pairs. This is a performance
- // heuristic, not a functional check.
- // TODO: Generalize this slightly to allow single instruction cases, and
- // prune the logic above which is mostly covered by this already.
+ // Also handles masked slides into an identity source, and single slides
+ // without masking. Avoid matching bit rotates (which are not also element
+ // rotates) as slide pairs. This is a performance heuristic, not a
+ // functional check.
std::pair<int, int> SrcInfo[2];
unsigned RotateAmt;
MVT RotateVT;
if (isMaskedSlidePair(Mask, SrcInfo) &&
- !isLegalBitRotate(Mask, VT, Subtarget, RotateVT, RotateAmt)) {
+ (isElementRotate(SrcInfo, NumElts) ||
+ !isLegalBitRotate(Mask, VT, Subtarget, RotateVT, RotateAmt))) {
SDValue Sources[2];
auto GetSourceFor = [&](const std::pair<int, int> &Info) {
int SrcIdx = Info.first;
@@ -5737,6 +5640,12 @@ static SDValue lowerVECTOR_SHUFFLE(SDValue Op, SelectionDAG &DAG,
RISCVVType::TAIL_AGNOSTIC);
};
+ if (SrcInfo[1].first == -1) {
+ SDValue Res = DAG.getUNDEF(ContainerVT);
+ Res = GetSlide(SrcInfo[0], TrueMask, Res);
+ return convertFromScalableVector(VT, Res, DAG, Subtarget);
+ }
+
// Build the mask. Note that vslideup unconditionally preserves elements
// below the slide amount in the destination, and thus those elements are
// undefined in the mask. If the mask ends up all true (or undef), it
@@ -6055,9 +5964,10 @@ bool RISCVTargetLowering::isShuffleMaskLegal(ArrayRef<int> M, EVT VT) const {
if (SVT.getScalarType() == MVT::i1)
return false;
+ std::pair<int, int> SrcInfo[2];
int Dummy1, Dummy2;
return ShuffleVectorInst::isReverseMask(M, NumElts) ||
- (isElementRotate(Dummy1, Dummy2, M) > 0) ||
+ (isMaskedSlidePair(M, SrcInfo) && isElementRotate(SrcInfo, NumElts)) ||
isInterleaveShuffle(M, SVT, Dummy1, Dummy2, Subtarget);
}
More information about the llvm-commits
mailing list