fixup! Address review comments

  - Change what getMask returns
  - Change the name of TLI hooks
  - Limit the TLI hooks to scalable vectors only for now

Author: mshockwave

llvm/include/llvm/CodeGen/TargetLowering.h

@@ -3158,12 +3158,10 @@ class TargetLoweringBase {
   ///
   /// \p Load is a vp.load instruction.
   /// \p Mask is a mask value
-  /// \p DeinterleaveIntrin is vector.deinterleave intrinsic
   /// \p DeinterleaveRes is a list of deinterleaved results.
   virtual bool
-  lowerInterleavedScalableLoad(VPIntrinsic *Load, Value *Mask,
-                               IntrinsicInst *DeinterleaveIntrin,
-                               ArrayRef<Value *> DeinterleaveRes) const {
+  lowerDeinterleavedIntrinsicToVPLoad(VPIntrinsic *Load, Value *Mask,
+                                      ArrayRef<Value *> DeinterleaveRes) const {
     return false;
   }
 
@@ -3172,12 +3170,10 @@ class TargetLoweringBase {
   ///
   /// \p Store is the vp.store instruction.
   /// \p Mask is a mask value
-  /// \p InterleaveIntrin is vector.interleave intrinsic
   /// \p InterleaveOps is a list of values being interleaved.
   virtual bool
-  lowerInterleavedScalableStore(VPIntrinsic *Store, Value *Mask,
-                                IntrinsicInst *InterleaveIntrin,
-                                ArrayRef<Value *> InterleaveOps) const {
+  lowerInterleavedIntrinsicToVPStore(VPIntrinsic *Store, Value *Mask,
+                                     ArrayRef<Value *> InterleaveOps) const {
     return false;
   }
 

llvm/lib/CodeGen/InterleavedAccessPass.cpp

@@ -637,7 +637,7 @@ getVectorDeinterleaveFactor(IntrinsicInst *II,
 /// - if a value within the option is nullptr, the value corresponds to all-true
 ///   mask
 /// - return nullopt if mask cannot be deinterleaved
-static std::optional<Value *> getMask(Value *WideMask, unsigned Factor) {
+static Value *getMask(Value *WideMask, unsigned Factor) {
   using namespace llvm::PatternMatch;
   if (auto *IMI = dyn_cast<IntrinsicInst>(WideMask)) {
     SmallVector<Value *, 8> Operands;
@@ -650,8 +650,9 @@ static std::optional<Value *> getMask(Value *WideMask, unsigned Factor) {
     }
   }
   if (match(WideMask, m_AllOnes()))
-    return nullptr;
-  return std::nullopt;
+    return WideMask;
+
+  return nullptr;
 }
 
 bool InterleavedAccessImpl::lowerDeinterleaveIntrinsic(
@@ -673,7 +674,7 @@ bool InterleavedAccessImpl::lowerDeinterleaveIntrinsic(
       return false;
     // Check mask operand. Handle both all-true and interleaved mask.
     Value *WideMask = VPLoad->getOperand(1);
-    std::optional<Value *> Mask = getMask(WideMask, Factor);
+    Value *Mask = getMask(WideMask, Factor);
     if (!Mask)
       return false;
 
@@ -682,8 +683,8 @@ bool InterleavedAccessImpl::lowerDeinterleaveIntrinsic(
 
     // Since lowerInterleaveLoad expects Shuffles and LoadInst, use special
     // TLI function to emit target-specific interleaved instruction.
-    if (!TLI->lowerInterleavedScalableLoad(VPLoad, *Mask, DI,
-                                           DeinterleaveValues))
+    if (!TLI->lowerDeinterleavedIntrinsicToVPLoad(VPLoad, Mask,
+                                                  DeinterleaveValues))
       return false;
 
   } else {
@@ -725,7 +726,7 @@ bool InterleavedAccessImpl::lowerInterleaveIntrinsic(
       return false;
 
     Value *WideMask = VPStore->getOperand(2);
-    std::optional<Value *> Mask = getMask(WideMask, Factor);
+    Value *Mask = getMask(WideMask, Factor);
     if (!Mask)
       return false;
 
@@ -734,8 +735,8 @@ bool InterleavedAccessImpl::lowerInterleaveIntrinsic(
 
     // Since lowerInterleavedStore expects Shuffle and StoreInst, use special
     // TLI function to emit target-specific interleaved instruction.
-    if (!TLI->lowerInterleavedScalableStore(VPStore, *Mask, II,
-                                            InterleaveValues))
+    if (!TLI->lowerInterleavedIntrinsicToVPStore(VPStore, Mask,
+                                                 InterleaveValues))
       return false;
   } else {
     auto *SI = cast<StoreInst>(StoredBy);

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

@@ -22555,15 +22555,20 @@ bool RISCVTargetLowering::lowerInterleaveIntrinsicToStore(
 ///
 /// NOTE: the deinterleave2 intrinsic won't be touched and is expected to be
 /// removed by the caller
-bool RISCVTargetLowering::lowerInterleavedScalableLoad(
-    VPIntrinsic *Load, Value *Mask, IntrinsicInst *DeinterleaveIntrin,
+bool RISCVTargetLowering::lowerDeinterleavedIntrinsicToVPLoad(
+    VPIntrinsic *Load, Value *Mask,
     ArrayRef<Value *> DeInterleaveResults) const {
+  assert(Mask && "Expect a valid mask");
   assert(Load->getIntrinsicID() == Intrinsic::vp_load &&
          "Unexpected intrinsic");
 
   const unsigned Factor = DeInterleaveResults.size();
 
-  auto *WideVTy = cast<VectorType>(Load->getType());
+  auto *WideVTy = dyn_cast<ScalableVectorType>(Load->getType());
+  // TODO: Support fixed vectors.
+  if (!WideVTy)
+    return false;
+
   unsigned WideNumElements = WideVTy->getElementCount().getKnownMinValue();
   assert(WideNumElements % Factor == 0 &&
          "ElementCount of a wide load must be divisible by interleave factor");
@@ -22605,10 +22610,6 @@ bool RISCVTargetLowering::lowerInterleavedScalableLoad(
   SmallVector<Value *> Operands;
   Operands.append({PoisonVal, Load->getArgOperand(0)});
 
-  if (!Mask)
-    Mask = ConstantVector::getSplat(VTy->getElementCount(),
-                                    ConstantInt::getTrue(Load->getContext()));
-
   Function *VlsegNFunc = Intrinsic::getOrInsertDeclaration(
       Load->getModule(), IntrMaskIds[Factor - 2],
       {VecTupTy, Mask->getType(), EVL->getType()});
@@ -22653,16 +22654,12 @@ bool RISCVTargetLowering::lowerInterleavedScalableLoad(
 /// into
 /// `<vscale x 8 x i32>`. This will resuling a simple unit stride store rather
 /// than a segment store, which is more expensive in this case.
-static Value *foldInterleaved2OfConstSplats(IntrinsicInst *InterleaveIntrin,
+static Value *foldInterleaved2OfConstSplats(Value *Op0, Value *Op1,
                                             VectorType *VTy,
                                             const TargetLowering *TLI,
                                             Instruction *VPStore) {
-  // We only handle Factor = 2 for now.
-  assert(InterleaveIntrin->arg_size() == 2);
-  auto *SplatVal0 = dyn_cast_or_null<ConstantInt>(
-      getSplatValue(InterleaveIntrin->getArgOperand(0)));
-  auto *SplatVal1 = dyn_cast_or_null<ConstantInt>(
-      getSplatValue(InterleaveIntrin->getArgOperand(1)));
+  auto *SplatVal0 = dyn_cast_or_null<ConstantInt>(getSplatValue(Op0));
+  auto *SplatVal1 = dyn_cast_or_null<ConstantInt>(getSplatValue(Op1));
   if (!SplatVal0 || !SplatVal1)
     return nullptr;
 
@@ -22711,15 +22708,19 @@ static Value *foldInterleaved2OfConstSplats(IntrinsicInst *InterleaveIntrin,
 ///                               <vscale x 32 x i8> %load2, ptr %ptr,
 ///                               %mask,
 ///                               i64 %rvl)
-bool RISCVTargetLowering::lowerInterleavedScalableStore(
-    VPIntrinsic *Store, Value *Mask, IntrinsicInst *InterleaveIntrin,
+bool RISCVTargetLowering::lowerInterleavedIntrinsicToVPStore(
+    VPIntrinsic *Store, Value *Mask,
     ArrayRef<Value *> InterleaveOperands) const {
+  assert(Mask && "Expect a valid mask");
   assert(Store->getIntrinsicID() == Intrinsic::vp_store &&
          "Unexpected intrinsic");
 
   const unsigned Factor = InterleaveOperands.size();
 
-  VectorType *VTy = cast<VectorType>(InterleaveOperands[0]->getType());
+  auto *VTy = dyn_cast<ScalableVectorType>(InterleaveOperands[0]->getType());
+  // TODO: Support fixed vectors.
+  if (!VTy)
+    return false;
 
   // FIXME: Should pass alignment attribute from pointer, but vectorizer needs
   // to emit it first.
@@ -22731,9 +22732,10 @@ bool RISCVTargetLowering::lowerInterleavedScalableStore(
     return false;
 
   if (Factor == 2)
-    if (Value *BC =
-            foldInterleaved2OfConstSplats(InterleaveIntrin, VTy, this, Store)) {
-      InterleaveIntrin->replaceAllUsesWith(BC);
+    if (Value *BC = foldInterleaved2OfConstSplats(
+            InterleaveOperands[0], InterleaveOperands[1], VTy, this, Store)) {
+      // Store is guranteed to be the only user of the interleaved intrinsic.
+      Store->getOperand(0)->replaceAllUsesWith(BC);
       return true;
     }
 
@@ -22770,10 +22772,6 @@ bool RISCVTargetLowering::lowerInterleavedScalableStore(
   Operands.push_back(StoredVal);
   Operands.push_back(Store->getArgOperand(1));
 
-  if (!Mask)
-    Mask = ConstantVector::getSplat(VTy->getElementCount(),
-                                    ConstantInt::getTrue(Store->getContext()));
-
   Function *VssegNFunc = Intrinsic::getOrInsertDeclaration(
       Store->getModule(), IntrMaskIds[Factor - 2],
       {VecTupTy, Mask->getType(), EVL->getType()});

llvm/lib/Target/RISCV/RISCVISelLowering.h

@@ -910,14 +910,13 @@ class RISCVTargetLowering : public TargetLowering {
   bool lowerInterleaveIntrinsicToStore(
       StoreInst *SI, ArrayRef<Value *> InterleaveValues) const override;
 
-  bool lowerInterleavedScalableLoad(
-      VPIntrinsic *Load, Value *Mask, IntrinsicInst *DeinterleaveIntrin,
+  bool lowerDeinterleavedIntrinsicToVPLoad(
+      VPIntrinsic *Load, Value *Mask,
       ArrayRef<Value *> DeinterleaveRes) const override;
 
-  bool
-  lowerInterleavedScalableStore(VPIntrinsic *Store, Value *Mask,
-                                IntrinsicInst *InterleaveIntrin,
-                                ArrayRef<Value *> InterleaveOps) const override;
+  bool lowerInterleavedIntrinsicToVPStore(
+      VPIntrinsic *Store, Value *Mask,
+      ArrayRef<Value *> InterleaveOps) const override;
 
   bool supportKCFIBundles() const override { return true; }