[llvm] [RISCV] Recurse on second operand of two operand shuffles (PR #79197)
Philip Reames via llvm-commits
llvm-commits at lists.llvm.org
Wed Jan 24 08:29:07 PST 2024
================
@@ -5046,50 +5038,13 @@ static SDValue lowerVECTOR_SHUFFLE(SDValue Op, SelectionDAG &DAG,
return convertFromScalableVector(VT, Gather, DAG, Subtarget);
}
- // Translate the gather index we computed above (and possibly swapped)
- // back to a shuffle mask. This step should disappear once we complete
- // the migration to recursive design.
- SmallVector<int> ShuffleMaskLHS;
- ShuffleMaskLHS.reserve(GatherIndicesLHS.size());
- for (SDValue GatherIndex : GatherIndicesLHS) {
- if (GatherIndex.isUndef()) {
- ShuffleMaskLHS.push_back(-1);
- continue;
- }
- auto *IdxC = cast<ConstantSDNode>(GatherIndex);
- ShuffleMaskLHS.push_back(IdxC->getZExtValue());
- }
-
- // Recursively invoke lowering for the LHS as if there were no RHS.
- // This allows us to leverage all of our single source permute tricks.
- SDValue Gather =
- DAG.getVectorShuffle(VT, DL, V1, DAG.getUNDEF(VT), ShuffleMaskLHS);
- Gather = convertToScalableVector(ContainerVT, Gather, DAG, Subtarget);
-
- // Blend in second vector source with an additional vrgather.
- V2 = convertToScalableVector(ContainerVT, V2, DAG, Subtarget);
-
- MVT MaskContainerVT = ContainerVT.changeVectorElementType(MVT::i1);
- SelectMask =
- convertToScalableVector(MaskContainerVT, SelectMask, DAG, Subtarget);
-
- // If only one index is used, we can use a "splat" vrgather.
- // TODO: We can splat the most-common index and fix-up any stragglers, if
- // that's beneficial.
- if (RHSIndexCounts.size() == 1) {
- int SplatIndex = RHSIndexCounts.begin()->getFirst();
- Gather = DAG.getNode(GatherVXOpc, DL, ContainerVT, V2,
- DAG.getConstant(SplatIndex, DL, XLenVT), Gather,
- SelectMask, VL);
- } else {
- SDValue RHSIndices = DAG.getBuildVector(IndexVT, DL, GatherIndicesRHS);
- RHSIndices =
- convertToScalableVector(IndexContainerVT, RHSIndices, DAG, Subtarget);
- Gather = DAG.getNode(GatherVVOpc, DL, ContainerVT, V2, RHSIndices, Gather,
- SelectMask, VL);
- }
-
- return convertFromScalableVector(VT, Gather, DAG, Subtarget);
+ // Recursively invoke lowering for each operand if we had two
+ // independent single source permutes, and then combine the result via a
+ // vselect. Note that the vselect will likely be folded back into the
+ // second permute (vrgather, or other) by the post-isel combine.
+ V1 = DAG.getVectorShuffle(VT, DL, V1, DAG.getUNDEF(VT), ShuffleMaskLHS);
+ V2 = DAG.getVectorShuffle(VT, DL, V2, DAG.getUNDEF(VT), ShuffleMaskRHS);
+ return DAG.getNode(ISD::VSELECT, DL, VT, SelectMask, V2, V1);
----------------
preames wrote:
Part of the point of this patch series was to better handle identity permutes. :)
Your example looks less like a case where we fail to realize one side is a identity permute, and more like we fail to realize that the vmerge can be folded back into the LHS in this case.
One simple thing we could do - extend the swap logic to prefer the identity on the LHS. This feels more like hiding the real issue than a deep fix, but it wouldn't be hard to do.
Can you file a bug for this? I don't plan to look at it immediately, but it'd be good not to forget it.
https://github.com/llvm/llvm-project/pull/79197
More information about the llvm-commits
mailing list