[llvm] 4e1ba0c - [RISCV] Don't accidentally match deinterleave masks as interleaves

[Luke Lau via llvm-commits]

Author: Luke Lau
Date: 2023-03-16T15:48:51Z
New Revision: 4e1ba0c518687b44ea76eed65581c9e3dbacbb40

URL: https://github.com/llvm/llvm-project/commit/4e1ba0c518687b44ea76eed65581c9e3dbacbb40
DIFF: https://github.com/llvm/llvm-project/commit/4e1ba0c518687b44ea76eed65581c9e3dbacbb40.diff

LOG: [RISCV] Don't accidentally match deinterleave masks as interleaves

Consider a shuffle mask of <0, 2>:
This is one of two deinterleave masks to deinterleave a vector of 4
elements with factor 2.
Unfortunately, this is also technically an interleave mask, where
two subvectors of length 1 at indexes 0 and 2 will be interleaved.
This is because a mask can interleave non-contiguous subvectors:
e.g. <0, 6, 4, 1, 7, 5> on a vector of size 8:

```
<0 1 2 3 4 5 6 7> indices
 ^ ^     ^ ^ ^ ^
 0 0     2 2 1 1  deinterleaved subvector
```

This means that deinterleaving shuffles can accidentally be costed as
interleaves.
And it's incorrect in the context of interleaves, because the
only interleave shuffles we model at the moment are single permutation
shuffles, i.e. we are interleaving the first vector below and ignoring
the second:

shufflevector <2 x i32> %v0, <2 x i32> poison, <2 x i32> <i32 0, i32 2>

A mask of <0, 2> interleaves across both vectors.

The fix here is to set NumInputElts correctly: We were setting it to
twice the mask length, i.e. using both input vectors. But in fact we're
actually only using the first vector here, and isInterleaveMask actually
already has logic to ensure that the mask indices stay within the bounds
of the input vectors.

This lacks a test case due to how we're unable to test deinterleave
shuffles (because they are length changing), but is covered in the tests
in D145155

Reviewed By: reames

Differential Revision: https://reviews.llvm.org/D146176

Added: 
    

Modified: 
    llvm/include/llvm/IR/Instructions.h
    llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/IR/Instructions.h b/llvm/include/llvm/IR/Instructions.h
index ee38a118f182d..46202e4570ff4 100644
--- a/llvm/include/llvm/IR/Instructions.h
+++ b/llvm/include/llvm/IR/Instructions.h
@@ -2448,6 +2448,10 @@ class ShuffleVectorInst : public Instruction {
   /// StartIndexes are the first indexes of each vector being interleaved,
   /// substituting any indexes that were undef
   /// E.g. <4, -1, 2, 5, 1, 3> (Factor=3): StartIndexes=<4, 0, 2>
+  ///
+  /// Note that this does not check if the input vectors are consecutive:
+  /// It will return true for masks such as
+  /// <0, 4, 6, 1, 5, 7> (Factor=3, LaneLen=2)
   static bool isInterleaveMask(ArrayRef<int> Mask, unsigned Factor,
                                unsigned NumInputElts,
                                SmallVectorImpl<unsigned> &StartIndexes);

diff  --git a/llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp b/llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
index 23bc4b7aed5bc..2c4618a1ce6e9 100644
--- a/llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
@@ -264,13 +264,12 @@ InstructionCost RISCVTTIImpl::getShuffleCost(TTI::ShuffleKind Kind,
         // deinterleaves of 2 vectors can be lowered into the following
         // sequences
         if (EltTp.getScalarSizeInBits() < ST->getELEN()) {
-          auto InterleaveMask = createInterleaveMask(Mask.size() / 2, 2);
           // Example sequence:
           //   vsetivli     zero, 4, e8, mf4, ta, ma (ignored)
           //   vwaddu.vv    v10, v8, v9
           //   li       a0, -1                   (ignored)
           //   vwmaccu.vx   v10, a0, v9
-          if (ShuffleVectorInst::isInterleaveMask(Mask, 2, Mask.size() * 2))
+          if (ShuffleVectorInst::isInterleaveMask(Mask, 2, Mask.size()))
             return 2 * LT.first * getLMULCost(LT.second);
 
           if (Mask[0] == 0 || Mask[0] == 1) {