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

[Luke Lau via Phabricator via llvm-commits]

luke created this revision.
luke added a reviewer: reames.
Herald added subscribers: jobnoorman, asb, pmatos, VincentWu, vkmr, frasercrmck, evandro, luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, shiva0217, kito-cheng, niosHD, sabuasal, simoncook, johnrusso, rbar, hiraditya, arichardson.
Herald added a project: All.
luke requested review of this revision.
Herald added subscribers: llvm-commits, pcwang-thead, eopXD, MaskRay.
Herald added a project: LLVM.

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 <https://reviews.llvm.org/D145155>


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146176

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


Index: llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
===================================================================
--- llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
+++ llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
@@ -264,13 +264,12 @@
         // 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) {
Index: llvm/include/llvm/IR/Instructions.h
===================================================================
--- llvm/include/llvm/IR/Instructions.h
+++ llvm/include/llvm/IR/Instructions.h
@@ -2448,6 +2448,10 @@
   /// 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);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D146176.505647.patch
Type: text/x-patch
Size: 1764 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230315/bd5c1f9c/attachment.bin>