[PATCH] D145678: [RISCV] Model interleave and deinterleave shuffles in cost model

Luke Lau via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 9 09:02:44 PST 2023


luke marked an inline comment as not done.
luke added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp:331
+    std::pair<InstructionCost, MVT> LT = getTypeLegalizationCost(Tp);
+    if (LT.second.isFixedLengthVector()) {
+      MVT EltTp = LT.second.getVectorElementType();
----------------
reames wrote:
> I think that LT.second must be fixed if the original type was.  Turn this into an assert?
I thought so too, but <2 x i64> can get scalarized to i64 on zve32x! The zve32x test case in https://reviews.llvm.org/D145697 exercises this


================
Comment at: llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp:343
+        //   vwmaccu.vx	v10, a0, v9
+        if (equal(InterleaveMask, Mask))
+          return 2 * LT.first * getLMULCost(LT.second);
----------------
reames wrote:
> Creating the masks just to check them this way is a bit inefficient.  I'm fine with this landing as is, but we probably need to generalize out isInterleaveMask utilities.  We already have that code in the lowering, so this is at least our second copy.  
> 
> Follow up change is fine.
I agree, I was hoping there would be something equivalent to `isReverse()`/`isTransposeMask()` in `ShuffleVectorInst`. 
I was also wondering if there should also be a dedicated `ShuffleKind` for this like `SK_[DE]INTERLEAVE`. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145678



More information about the llvm-commits mailing list