Skip to content

[RISCV][TTI] Fix a misuse of the getShuffleCost API [NFC] #129137

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 28, 2025

Conversation

preames
Copy link
Collaborator

@preames preames commented Feb 27, 2025

The getShuffleCost api, in concept, expects to only deal with non-length changing shuffles. We were failing to extend the mask appropriately before invoking it. This came up in #128537 in discussion of a potential invariant, but is otherwise unrelated.

The getShuffleCost api expects to only deal with non-length changing
shuffles.  We were failing to extend the mask appropriately before
invoking it.  This came up in llvm#128537
in discussion of a potential invariant, but is otherwise unrelated.
@llvmbot
Copy link
Member

llvmbot commented Feb 27, 2025

@llvm/pr-subscribers-backend-risc-v

Author: Philip Reames (preames)

Changes

The getShuffleCost api expects to only deal with non-length changing shuffles. We were failing to extend the mask appropriately before invoking it. This came up in #128537 in discussion of a potential invariant, but is otherwise unrelated.


Full diff: https://github.com/llvm/llvm-project/pull/129137.diff

1 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp (+3-3)
diff --git a/llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp b/llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
index dfea25e11c0b6..31183dd8a037c 100644
--- a/llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
@@ -481,7 +481,6 @@ InstructionCost RISCVTTIImpl::getShuffleCost(TTI::ShuffleKind Kind,
                                              ArrayRef<const Value *> Args,
                                              const Instruction *CxtI) {
   Kind = improveShuffleKindFromMask(Kind, Mask, Tp, Index, SubTp);
-
   std::pair<InstructionCost, MVT> LT = getTypeLegalizationCost(Tp);
 
   // First, handle cases where having a fixed length vector enables us to
@@ -889,11 +888,12 @@ InstructionCost RISCVTTIImpl::getInterleavedMemoryOpCost(
   if (Opcode == Instruction::Load) {
     InstructionCost Cost = MemCost;
     for (unsigned Index : Indices) {
-      FixedVectorType *SubVecTy =
+      FixedVectorType *VecTy =
           FixedVectorType::get(FVTy->getElementType(), VF * Factor);
       auto Mask = createStrideMask(Index, Factor, VF);
+      Mask.resize(VF * Factor, -1);
       InstructionCost ShuffleCost =
-          getShuffleCost(TTI::ShuffleKind::SK_PermuteSingleSrc, SubVecTy, Mask,
+          getShuffleCost(TTI::ShuffleKind::SK_PermuteSingleSrc, VecTy, Mask,
                          CostKind, 0, nullptr, {});
       Cost += ShuffleCost;
     }

@preames preames merged commit 1bd13bc into llvm:main Feb 28, 2025
13 checks passed
@preames preames deleted the pr-riscv-tti-interleave-shuffle-cost-fix branch February 28, 2025 02:53
cheezeburglar pushed a commit to cheezeburglar/llvm-project that referenced this pull request Feb 28, 2025
The getShuffleCost api, in concept, expects to only deal with non-length
changing shuffles. We were failing to extend the mask appropriately
before invoking it. This came up in
llvm#128537 in discussion of a
potential invariant, but is otherwise unrelated.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants