[llvm] 37cf4fc - [RISCV] Consolidate legality checking for strided load/store [nfc]

[Philip Reames via llvm-commits]

Author: Philip Reames
Date: 2023-04-28T08:13:03-07:00
New Revision: 37cf4fc6cef9f2f50e15622a1bdecc89ea6ddd23

URL: https://github.com/llvm/llvm-project/commit/37cf4fc6cef9f2f50e15622a1bdecc89ea6ddd23
DIFF: https://github.com/llvm/llvm-project/commit/37cf4fc6cef9f2f50e15622a1bdecc89ea6ddd23.diff

LOG: [RISCV] Consolidate legality checking for strided load/store [nfc]

Note that the strided load from concat_vector combine was using the wrong legality test. It happened to work out as the alignment requirement is based on the scalar type either way, but unless I'm missing something allowsMisalignedAccess is expecting a contiguous memory access.

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

Added: 
    

Modified: 
    llvm/lib/Target/RISCV/RISCVGatherScatterLowering.cpp
    llvm/lib/Target/RISCV/RISCVISelLowering.cpp
    llvm/lib/Target/RISCV/RISCVISelLowering.h

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/RISCV/RISCVGatherScatterLowering.cpp b/llvm/lib/Target/RISCV/RISCVGatherScatterLowering.cpp
index d40ad25e2875..590ea5ec7df4 100644
--- a/llvm/lib/Target/RISCV/RISCVGatherScatterLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVGatherScatterLowering.cpp
@@ -63,8 +63,6 @@ class RISCVGatherScatterLowering : public FunctionPass {
   }
 
 private:
-  bool isLegalTypeAndAlignment(Type *DataType, Value *AlignOp);
-
   bool tryCreateStridedLoadStore(IntrinsicInst *II, Type *DataType, Value *Ptr,
                                  Value *AlignOp);
 
@@ -87,24 +85,6 @@ FunctionPass *llvm::createRISCVGatherScatterLoweringPass() {
   return new RISCVGatherScatterLowering();
 }
 
-bool RISCVGatherScatterLowering::isLegalTypeAndAlignment(Type *DataType,
-                                                         Value *AlignOp) {
-  Type *ScalarType = DataType->getScalarType();
-  if (!TLI->isLegalElementTypeForRVV(ScalarType))
-    return false;
-
-  MaybeAlign MA = cast<ConstantInt>(AlignOp)->getMaybeAlignValue();
-  if (MA && MA->value() < DL->getTypeStoreSize(ScalarType).getFixedValue())
-    return false;
-
-  // FIXME: Let the backend type legalize by splitting/widening?
-  EVT DataVT = TLI->getValueType(*DL, DataType);
-  if (!TLI->isTypeLegal(DataVT))
-    return false;
-
-  return true;
-}
-
 // TODO: Should we consider the mask when looking for a stride?
 static std::pair<Value *, Value *> matchStridedConstant(Constant *StartC) {
   if (!isa<FixedVectorType>(StartC->getType()))
@@ -464,7 +444,12 @@ bool RISCVGatherScatterLowering::tryCreateStridedLoadStore(IntrinsicInst *II,
                                                            Value *Ptr,
                                                            Value *AlignOp) {
   // Make sure the operation will be supported by the backend.
-  if (!isLegalTypeAndAlignment(DataType, AlignOp))
+  MaybeAlign MA = cast<ConstantInt>(AlignOp)->getMaybeAlignValue();
+  if (!MA || !TLI->isLegalStridedLoadStore(*DL, DataType, *MA))
+    return false;
+
+  // FIXME: Let the backend type legalize by splitting/widening?
+  if (!TLI->isTypeLegal(TLI->getValueType(*DL, DataType)))
     return false;
 
   // Pointer should be a GEP.

diff  --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
index aae1e07bc41d..a9845fe22f8d 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -11426,6 +11426,11 @@ static SDValue performCONCAT_VECTORSCombine(SDNode *N, SelectionDAG &DAG,
   if (!TLI.isTypeLegal(WideVecVT))
     return SDValue();
 
+  // Check that the operation is legal
+  Type *WideVecTy = EVT(WideVecVT).getTypeForEVT(*DAG.getContext());
+  if (!TLI.isLegalStridedLoadStore(DAG.getDataLayout(), WideVecTy, Align))
+    return SDValue();
+
   MVT ContainerVT = TLI.getContainerForFixedLengthVector(WideVecVT);
   SDValue VL =
       getDefaultVLOps(WideVecVT, ContainerVT, DL, DAG, Subtarget).second;
@@ -11453,12 +11458,6 @@ static SDValue performCONCAT_VECTORSCombine(SDNode *N, SelectionDAG &DAG,
       BaseLd->getPointerInfo(), BaseLd->getMemOperand()->getFlags(), MemSize,
       Align);
 
-  // Can't do the combine if the common alignment isn't naturally aligned with
-  // the new element type
-  if (!TLI.allowsMemoryAccessForAlignment(*DAG.getContext(),
-                                          DAG.getDataLayout(), WideVecVT, *MMO))
-    return SDValue();
-
   SDValue StridedLoad = DAG.getMemIntrinsicNode(ISD::INTRINSIC_W_CHAIN, DL, VTs,
                                                 Ops, WideVecVT, MMO);
   for (SDValue Ld : N->ops())
@@ -15798,6 +15797,26 @@ bool RISCVTargetLowering::isLegalInterleavedAccessType(
   return Factor * LMUL <= 8;
 }
 
+bool RISCVTargetLowering::isLegalStridedLoadStore(const DataLayout &DL,
+                                                  Type *DataType,
+                                                  Align Alignment) const {
+  if (!Subtarget.hasVInstructions())
+    return false;
+
+  // Only support fixed vectors if we know the minimum vector size.
+  if (isa<FixedVectorType>(DataType) && !Subtarget.useRVVForFixedLengthVectors())
+    return false;
+
+  Type *ScalarType = DataType->getScalarType();
+  if (!isLegalElementTypeForRVV(ScalarType))
+    return false;
+
+  if (Alignment < DL.getTypeStoreSize(ScalarType).getFixedValue())
+    return false;
+
+  return true;
+}
+
 /// Lower an interleaved load into a vlsegN intrinsic.
 ///
 /// E.g. Lower an interleaved load (Factor = 2):

diff  --git a/llvm/lib/Target/RISCV/RISCVISelLowering.h b/llvm/lib/Target/RISCV/RISCVISelLowering.h
index 5dada8a43819..849b11f5b000 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.h
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.h
@@ -704,6 +704,10 @@ class RISCVTargetLowering : public TargetLowering {
   bool isLegalInterleavedAccessType(FixedVectorType *, unsigned Factor,
                                     const DataLayout &) const;
 
+  /// Return true if a stride load store of the given result type and
+  /// alignment is legal.
+  bool isLegalStridedLoadStore(const DataLayout &DL, Type *DataType, Align Alignment) const;
+
   unsigned getMaxSupportedInterleaveFactor() const override { return 8; }
 
   bool lowerInterleavedLoad(LoadInst *LI,