[RISCV] Fix crash during during i1 vector bitreverse lowering

[luke]

Summary

A shuffle of v256i1 with a large enough minimum vlen might make it through type
legalization and into lowering. In this case, zvl1024b was enough. The
bitreverse shuffle lowering would then try to convert this to a v1i256 type
which is invalid (v1i128 exists though, which is why the existing v128i1 tests
were fine).

This patch checks to make sure that the new type is not only legal but also
valid.

[wangpc]

I was thinking about adding v1i256, v1i512, etc. Is it feasible?

[craig.topper]

llvm/test/CodeGen/RISCV/rvv/fixed-vectors-shuffle-reverse.ll

5 ↗

(On Diff #554768)

This is the wrong command line for the bug. Don't you need +zvl256b? The bug is due to the min vlen, not the max vlen.

[craig.topper]

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

4229–4230

Can we use EVT ViaVT = EVT::getVectorVT(*DAG.getContext(), EVT::getIntegerVT(*DAG.getContext(), ViaEltSize), 1); so we're not trying to create non-existent MVTs?

[luke]

Use EVTs (should future proof against larger types if they're ever added
too)
Fix test, move it into separate file since changing zvl causes a large
test diff, as all the LMULs change

[luke]

In D159215#4630041, @wangpc wrote:

I was thinking about adding v1i256, v1i512, etc. Is it feasible?

I've updated the diff to use EVTs as per Craig's suggestion, it should be ok if any larger types are added now.

llvm/test/CodeGen/RISCV/rvv/fixed-vectors-shuffle-reverse.ll

5 ↗

(On Diff #554768)

Yeah, not sure what I was thinking.

[luke]

Add a comment to the test case

[reames]

LGTM

[craig.topper]

LGTM

llvm/test/CodeGen/RISCV/rvv/fixed-vectors-shuffle-reverse-bitrotate.ll

5

corresponding*

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

static SDValue lowerBitreverseShuffle(ShuffleVectorSDNode *SVN,
   unsigned NumElts = VT.getVectorNumElements();
 
   assert(VT.getVectorElementType() == MVT::i1);
 
   if (!ShuffleVectorInst::isReverseMask(SVN->getMask()) ||
       !SVN->getOperand(1).isUndef())
     return SDValue();
 
   unsigned ViaEltSize = std::max((uint64_t)8, PowerOf2Ceil(NumElts));
-  MVT ViaVT = MVT::getVectorVT(MVT::getIntegerVT(ViaEltSize), 1);
-  MVT ViaBitVT = MVT::getVectorVT(MVT::i1, ViaVT.getScalarSizeInBits());
+  EVT ViaVT = EVT::getVectorVT(
+      *DAG.getContext(), EVT::getIntegerVT(*DAG.getContext(), ViaEltSize), 1);
+  EVT ViaBitVT =
+      EVT::getVectorVT(*DAG.getContext(), MVT::i1, ViaVT.getScalarSizeInBits());
 
   // If we don't have zvbb or the larger element type > ELEN, the operation will
   // be illegal.
   if (!Subtarget.getTargetLowering()->isOperationLegalOrCustom(ISD::BITREVERSE,
-                                                               ViaVT))
+                                                               ViaVT) ||
+      !Subtarget.getTargetLowering()->isTypeLegal(ViaBitVT))
     return SDValue();
 
   // If the bit vector doesn't fit exactly into the larger element type, we need
   // to insert it into the larger vector and then shift up the reversed bits
   // afterwards to get rid of the gap introduced.
   if (ViaEltSize > NumElts)
     V = DAG.getNode(ISD::INSERT_SUBVECTOR, DL, ViaBitVT, DAG.getUNDEF(ViaBitVT),
                     V, DAG.getVectorIdxConstant(0, DL));

llvm/test/CodeGen/RISCV/rvv/fixed-vectors-shuffle-reverse-bitrotate.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 3
+; RUN: llc -mtriple=riscv32 -mattr=+v,+zvl1024b -verify-machineinstrs < %s | FileCheck %s
+; RUN: llc -mtriple=riscv64 -mattr=+v,+zvl1024b -verify-machineinstrs < %s | FileCheck %s
+
+; There is no correpsonding v1i256 type, so make sure we don't crash if we try
+; to lower via lowerBitreverseShuffle.
+define <256 x i1> @reverse_v256i1(<256 x i1> %a) {
+; CHECK-LABEL: reverse_v256i1:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    li a0, 256
+; CHECK-NEXT:    vsetvli zero, a0, e8, m2, ta, ma
+; CHECK-NEXT:    vmv.v.i v8, 0
+; CHECK-NEXT:    vmerge.vim v8, v8, 1, v0
+; CHECK-NEXT:    vid.v v10
+; CHECK-NEXT:    vrsub.vi v10, v10, -1
+; CHECK-NEXT:    vrgather.vv v12, v8, v10
+; CHECK-NEXT:    vmsne.vi v0, v12, 0
+; CHECK-NEXT:    ret
+  %res = call <256 x i1> @llvm.experimental.vector.reverse.v256i1(<256 x i1> %a)
+  ret <256 x i1> %res
+}
+
+declare <256 x i1> @llvm.experimental.vector.reverse.v256i1(<256 x i1>)