Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AArch64] Support scalable offsets with isLegalAddressingMode #83255

Merged

Conversation

huntergr-arm
Copy link
Collaborator

Allows us to indicate that an addressing mode featuring a vscale-relative immediate offset is supported.

See the RFC for reference: https://discourse.llvm.org/t/rfc-vscale-aware-loopstrengthreduce/77131

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 28, 2024

@llvm/pr-subscribers-backend-aarch64
@llvm/pr-subscribers-backend-webassembly
@llvm/pr-subscribers-backend-powerpc
@llvm/pr-subscribers-backend-arm
@llvm/pr-subscribers-backend-amdgpu
@llvm/pr-subscribers-backend-risc-v
@llvm/pr-subscribers-llvm-analysis

@llvm/pr-subscribers-backend-systemz

Author: Graham Hunter (huntergr-arm)

Changes

Allows us to indicate that an addressing mode featuring a vscale-relative immediate offset is supported.

See the RFC for reference: https://discourse.llvm.org/t/rfc-vscale-aware-loopstrengthreduce/77131


Patch is 21.17 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/83255.diff

23 Files Affected:

  • (modified) llvm/include/llvm/Analysis/TargetTransformInfo.h (+6-4)
  • (modified) llvm/include/llvm/Analysis/TargetTransformInfoImpl.h (+2-1)
  • (modified) llvm/include/llvm/CodeGen/BasicTTIImpl.h (+3-1)
  • (modified) llvm/include/llvm/CodeGen/TargetLowering.h (+1)
  • (modified) llvm/lib/Analysis/TargetTransformInfo.cpp (+3-2)
  • (modified) llvm/lib/CodeGen/TargetLoweringBase.cpp (+4)
  • (modified) llvm/lib/Target/AArch64/AArch64ISelLowering.cpp (+16)
  • (modified) llvm/lib/Target/AMDGPU/SIISelLowering.cpp (+4)
  • (modified) llvm/lib/Target/ARC/ARCISelLowering.cpp (+4)
  • (modified) llvm/lib/Target/ARM/ARMISelLowering.cpp (+4)
  • (modified) llvm/lib/Target/AVR/AVRISelLowering.cpp (+4)
  • (modified) llvm/lib/Target/BPF/BPFISelLowering.cpp (+4)
  • (modified) llvm/lib/Target/Hexagon/HexagonISelLowering.cpp (+4)
  • (modified) llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp (+4)
  • (modified) llvm/lib/Target/Mips/MipsISelLowering.cpp (+4)
  • (modified) llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp (+4)
  • (modified) llvm/lib/Target/PowerPC/PPCISelLowering.cpp (+4)
  • (modified) llvm/lib/Target/RISCV/RISCVISelLowering.cpp (+4)
  • (modified) llvm/lib/Target/SystemZ/SystemZISelLowering.cpp (+4)
  • (modified) llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp (+4)
  • (modified) llvm/lib/Target/X86/X86ISelLowering.cpp (+4)
  • (modified) llvm/lib/Target/XCore/XCoreISelLowering.cpp (+4)
  • (modified) llvm/unittests/Target/AArch64/AddressingModes.cpp (+45-1)
diff --git a/llvm/include/llvm/Analysis/TargetTransformInfo.h b/llvm/include/llvm/Analysis/TargetTransformInfo.h
index 58577a6b6eb5c0..0cfda084805209 100644
--- a/llvm/include/llvm/Analysis/TargetTransformInfo.h
+++ b/llvm/include/llvm/Analysis/TargetTransformInfo.h
@@ -711,7 +711,8 @@ class TargetTransformInfo {
   bool isLegalAddressingMode(Type *Ty, GlobalValue *BaseGV, int64_t BaseOffset,
                              bool HasBaseReg, int64_t Scale,
                              unsigned AddrSpace = 0,
-                             Instruction *I = nullptr) const;
+                             Instruction *I = nullptr,
+                             bool OffsetIsScalable = false) const;
 
   /// Return true if LSR cost of C1 is lower than C2.
   bool isLSRCostLess(const TargetTransformInfo::LSRCost &C1,
@@ -1839,7 +1840,8 @@ class TargetTransformInfo::Concept {
   virtual bool isLegalAddressingMode(Type *Ty, GlobalValue *BaseGV,
                                      int64_t BaseOffset, bool HasBaseReg,
                                      int64_t Scale, unsigned AddrSpace,
-                                     Instruction *I) = 0;
+                                     Instruction *I,
+                                     bool OffsetIsScalable) = 0;
   virtual bool isLSRCostLess(const TargetTransformInfo::LSRCost &C1,
                              const TargetTransformInfo::LSRCost &C2) = 0;
   virtual bool isNumRegsMajorCostOfLSR() = 0;
@@ -2300,9 +2302,9 @@ class TargetTransformInfo::Model final : public TargetTransformInfo::Concept {
   }
   bool isLegalAddressingMode(Type *Ty, GlobalValue *BaseGV, int64_t BaseOffset,
                              bool HasBaseReg, int64_t Scale, unsigned AddrSpace,
-                             Instruction *I) override {
+                             Instruction *I, bool OffsetIsScalable) override {
     return Impl.isLegalAddressingMode(Ty, BaseGV, BaseOffset, HasBaseReg, Scale,
-                                      AddrSpace, I);
+                                      AddrSpace, I, OffsetIsScalable);
   }
   bool isLSRCostLess(const TargetTransformInfo::LSRCost &C1,
                      const TargetTransformInfo::LSRCost &C2) override {
diff --git a/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h b/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
index 13379cc126a40c..dd9265be3eeeec 100644
--- a/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
+++ b/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
@@ -220,7 +220,8 @@ class TargetTransformInfoImplBase {
 
   bool isLegalAddressingMode(Type *Ty, GlobalValue *BaseGV, int64_t BaseOffset,
                              bool HasBaseReg, int64_t Scale, unsigned AddrSpace,
-                             Instruction *I = nullptr) const {
+                             Instruction *I = nullptr,
+                             bool OffsetIsScalable = false) const {
     // Guess that only reg and reg+reg addressing is allowed. This heuristic is
     // taken from the implementation of LSR.
     return !BaseGV && BaseOffset == 0 && (Scale == 0 || Scale == 1);
diff --git a/llvm/include/llvm/CodeGen/BasicTTIImpl.h b/llvm/include/llvm/CodeGen/BasicTTIImpl.h
index 61f6564e8cd79b..4bb0255c2bcf70 100644
--- a/llvm/include/llvm/CodeGen/BasicTTIImpl.h
+++ b/llvm/include/llvm/CodeGen/BasicTTIImpl.h
@@ -334,12 +334,14 @@ class BasicTTIImplBase : public TargetTransformInfoImplCRTPBase<T> {
 
   bool isLegalAddressingMode(Type *Ty, GlobalValue *BaseGV, int64_t BaseOffset,
                              bool HasBaseReg, int64_t Scale,
-                             unsigned AddrSpace, Instruction *I = nullptr) {
+                             unsigned AddrSpace, Instruction *I = nullptr,
+                             bool OffsetIsScalable = false) {
     TargetLoweringBase::AddrMode AM;
     AM.BaseGV = BaseGV;
     AM.BaseOffs = BaseOffset;
     AM.HasBaseReg = HasBaseReg;
     AM.Scale = Scale;
+    AM.OffsetIsScalable = OffsetIsScalable;
     return getTLI()->isLegalAddressingMode(DL, AM, Ty, AddrSpace, I);
   }
 
diff --git a/llvm/include/llvm/CodeGen/TargetLowering.h b/llvm/include/llvm/CodeGen/TargetLowering.h
index f2e00aab8d5da2..90bfa2983d2cb1 100644
--- a/llvm/include/llvm/CodeGen/TargetLowering.h
+++ b/llvm/include/llvm/CodeGen/TargetLowering.h
@@ -2696,6 +2696,7 @@ class TargetLoweringBase {
     int64_t      BaseOffs = 0;
     bool         HasBaseReg = false;
     int64_t      Scale = 0;
+    bool         OffsetIsScalable = false;
     AddrMode() = default;
   };
 
diff --git a/llvm/lib/Analysis/TargetTransformInfo.cpp b/llvm/lib/Analysis/TargetTransformInfo.cpp
index 1f11f0d7dd620e..a2562a7edebaf0 100644
--- a/llvm/lib/Analysis/TargetTransformInfo.cpp
+++ b/llvm/lib/Analysis/TargetTransformInfo.cpp
@@ -403,9 +403,10 @@ bool TargetTransformInfo::isLegalAddressingMode(Type *Ty, GlobalValue *BaseGV,
                                                 int64_t BaseOffset,
                                                 bool HasBaseReg, int64_t Scale,
                                                 unsigned AddrSpace,
-                                                Instruction *I) const {
+                                                Instruction *I,
+                                                bool OffsetIsScalable) const {
   return TTIImpl->isLegalAddressingMode(Ty, BaseGV, BaseOffset, HasBaseReg,
-                                        Scale, AddrSpace, I);
+                                        Scale, AddrSpace, I, OffsetIsScalable);
 }
 
 bool TargetTransformInfo::isLSRCostLess(const LSRCost &C1,
diff --git a/llvm/lib/CodeGen/TargetLoweringBase.cpp b/llvm/lib/CodeGen/TargetLoweringBase.cpp
index 646c0c345e54e0..cef358890252de 100644
--- a/llvm/lib/CodeGen/TargetLoweringBase.cpp
+++ b/llvm/lib/CodeGen/TargetLoweringBase.cpp
@@ -2008,6 +2008,10 @@ bool TargetLoweringBase::isLegalAddressingMode(const DataLayout &DL,
   // The default implementation of this implements a conservative RISCy, r+r and
   // r+i addr mode.
 
+  // Scalable offsets not supported
+  if (AM.OffsetIsScalable)
+    return false;
+
   // Allows a sign-extended 16-bit immediate field.
   if (AM.BaseOffs <= -(1LL << 16) || AM.BaseOffs >= (1LL << 16)-1)
     return false;
diff --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
index 3b92e95d7c2876..c3c5c6ddff4025 100644
--- a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -16374,6 +16374,18 @@ bool AArch64TargetLowering::isLegalAddressingMode(const DataLayout &DL,
 
   if (Ty->isScalableTy()) {
     if (isa<ScalableVectorType>(Ty)) {
+      // See if we have a foldable vscale-based offset, for vector types which
+      // are either legal or smaller than the minimum; more work will be
+      // required if we need to consider addressing for types which need
+      // legalization by splitting.
+      uint64_t VecNumBytes = DL.getTypeSizeInBits(Ty).getKnownMinValue() / 8;
+      if (AM.HasBaseReg && AM.BaseOffs != 0 && AM.OffsetIsScalable &&
+          !AM.Scale && (AM.BaseOffs % VecNumBytes == 0) && VecNumBytes <= 16 &&
+          isPowerOf2_64(VecNumBytes)) {
+        int64_t Idx = AM.BaseOffs / (int64_t)VecNumBytes;
+        return Idx >= -8 && Idx <= 7;
+      }
+
       uint64_t VecElemNumBytes =
           DL.getTypeSizeInBits(cast<VectorType>(Ty)->getElementType()) / 8;
       return AM.HasBaseReg && !AM.BaseOffs &&
@@ -16383,6 +16395,10 @@ bool AArch64TargetLowering::isLegalAddressingMode(const DataLayout &DL,
     return AM.HasBaseReg && !AM.BaseOffs && !AM.Scale;
   }
 
+  // No scalable offsets allowed for non-scalable types.
+  if (AM.OffsetIsScalable)
+    return false;
+
   // check reg + imm case:
   // i.e., reg + 0, reg + imm9, reg + SIZE_IN_BYTES * uimm12
   uint64_t NumBytes = 0;
diff --git a/llvm/lib/Target/AMDGPU/SIISelLowering.cpp b/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
index 84ef9679ab9563..f632467d3568f2 100644
--- a/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
+++ b/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
@@ -1531,6 +1531,10 @@ bool SITargetLowering::isLegalMUBUFAddressingMode(const AddrMode &AM) const {
 bool SITargetLowering::isLegalAddressingMode(const DataLayout &DL,
                                              const AddrMode &AM, Type *Ty,
                                              unsigned AS, Instruction *I) const {
+  // No scalable offsets allowed.
+  if (AM.OffsetIsScalable)
+    return false;
+
   // No global is ever allowed as a base.
   if (AM.BaseGV)
     return false;
diff --git a/llvm/lib/Target/ARC/ARCISelLowering.cpp b/llvm/lib/Target/ARC/ARCISelLowering.cpp
index 5dd343d97b80c2..8570406425b554 100644
--- a/llvm/lib/Target/ARC/ARCISelLowering.cpp
+++ b/llvm/lib/Target/ARC/ARCISelLowering.cpp
@@ -737,6 +737,10 @@ bool ARCTargetLowering::isLegalAddressingMode(const DataLayout &DL,
                                               const AddrMode &AM, Type *Ty,
                                               unsigned AS,
                                               Instruction *I) const {
+  // No scalable offsets allowed.
+  if (AM.OffsetIsScalable)
+    return false;
+
   return AM.Scale == 0;
 }
 
diff --git a/llvm/lib/Target/ARM/ARMISelLowering.cpp b/llvm/lib/Target/ARM/ARMISelLowering.cpp
index b98006ed0cb3f4..2c7a602b7c4c63 100644
--- a/llvm/lib/Target/ARM/ARMISelLowering.cpp
+++ b/llvm/lib/Target/ARM/ARMISelLowering.cpp
@@ -19645,6 +19645,10 @@ bool ARMTargetLowering::isLegalT1ScaledAddressingMode(const AddrMode &AM,
 bool ARMTargetLowering::isLegalAddressingMode(const DataLayout &DL,
                                               const AddrMode &AM, Type *Ty,
                                               unsigned AS, Instruction *I) const {
+  // No scalable offsets allowed.
+  if (AM.OffsetIsScalable)
+    return false;
+
   EVT VT = getValueType(DL, Ty, true);
   if (!isLegalAddressImmediate(AM.BaseOffs, VT, Subtarget))
     return false;
diff --git a/llvm/lib/Target/AVR/AVRISelLowering.cpp b/llvm/lib/Target/AVR/AVRISelLowering.cpp
index f91e77adb8f810..74fa09c409acfd 100644
--- a/llvm/lib/Target/AVR/AVRISelLowering.cpp
+++ b/llvm/lib/Target/AVR/AVRISelLowering.cpp
@@ -1058,6 +1058,10 @@ bool AVRTargetLowering::isLegalAddressingMode(const DataLayout &DL,
                                               const AddrMode &AM, Type *Ty,
                                               unsigned AS,
                                               Instruction *I) const {
+  // No scalable offsets allowed.
+  if (AM.OffsetIsScalable)
+    return false;
+
   int64_t Offs = AM.BaseOffs;
 
   // Allow absolute addresses.
diff --git a/llvm/lib/Target/BPF/BPFISelLowering.cpp b/llvm/lib/Target/BPF/BPFISelLowering.cpp
index 4d8ace7c1ece02..f6f91cd8ee9bf3 100644
--- a/llvm/lib/Target/BPF/BPFISelLowering.cpp
+++ b/llvm/lib/Target/BPF/BPFISelLowering.cpp
@@ -920,6 +920,10 @@ bool BPFTargetLowering::isLegalAddressingMode(const DataLayout &DL,
                                               const AddrMode &AM, Type *Ty,
                                               unsigned AS,
                                               Instruction *I) const {
+  // No scalable offsets allowed.
+  if (AM.OffsetIsScalable)
+    return false;
+
   // No global is ever allowed as a base.
   if (AM.BaseGV)
     return false;
diff --git a/llvm/lib/Target/Hexagon/HexagonISelLowering.cpp b/llvm/lib/Target/Hexagon/HexagonISelLowering.cpp
index 13691053ddd707..a9f83f03f0c74d 100644
--- a/llvm/lib/Target/Hexagon/HexagonISelLowering.cpp
+++ b/llvm/lib/Target/Hexagon/HexagonISelLowering.cpp
@@ -3659,6 +3659,10 @@ bool HexagonTargetLowering::isFPImmLegal(const APFloat &Imm, EVT VT,
 bool HexagonTargetLowering::isLegalAddressingMode(const DataLayout &DL,
                                                   const AddrMode &AM, Type *Ty,
                                                   unsigned AS, Instruction *I) const {
+  // No scalable offsets allowed.
+  if (AM.OffsetIsScalable)
+    return false;
+
   if (Ty->isSized()) {
     // When LSR detects uses of the same base address to access different
     // types (e.g. unions), it will assume a conservative type for these
diff --git a/llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp b/llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp
index 3324dd2e8fc217..24e44cf398684d 100644
--- a/llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp
+++ b/llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp
@@ -4895,6 +4895,10 @@ bool LoongArchTargetLowering::isLegalAddressingMode(const DataLayout &DL,
   //  4. reg1 + reg2
   // TODO: Add more checks after support vector extension.
 
+  // No scalable offsets allowed.
+  if (AM.OffsetIsScalable)
+    return false;
+
   // No global is ever allowed as a base.
   if (AM.BaseGV)
     return false;
diff --git a/llvm/lib/Target/Mips/MipsISelLowering.cpp b/llvm/lib/Target/Mips/MipsISelLowering.cpp
index 97e830cec27cad..c35b61d22bfe5a 100644
--- a/llvm/lib/Target/Mips/MipsISelLowering.cpp
+++ b/llvm/lib/Target/Mips/MipsISelLowering.cpp
@@ -4292,6 +4292,10 @@ bool MipsTargetLowering::isLegalAddressingMode(const DataLayout &DL,
                                                const AddrMode &AM, Type *Ty,
                                                unsigned AS,
                                                Instruction *I) const {
+  // No scalable offsets allowed.
+  if (AM.OffsetIsScalable)
+    return false;
+
   // No global is ever allowed as a base.
   if (AM.BaseGV)
     return false;
diff --git a/llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp b/llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp
index 66a101036f9134..a4d61511d9b7c2 100644
--- a/llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp
+++ b/llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp
@@ -5092,6 +5092,10 @@ bool NVPTXTargetLowering::isLegalAddressingMode(const DataLayout &DL,
   // - [areg+immoff]
   // - [immAddr]
 
+  // No scalable offsets allowed.
+  if (AM.OffsetIsScalable)
+    return false;
+
   if (AM.BaseGV) {
     return !AM.BaseOffs && !AM.HasBaseReg && !AM.Scale;
   }
diff --git a/llvm/lib/Target/PowerPC/PPCISelLowering.cpp b/llvm/lib/Target/PowerPC/PPCISelLowering.cpp
index 51becf1d5b8584..1a2d07db850bb4 100644
--- a/llvm/lib/Target/PowerPC/PPCISelLowering.cpp
+++ b/llvm/lib/Target/PowerPC/PPCISelLowering.cpp
@@ -16814,6 +16814,10 @@ bool PPCTargetLowering::isLegalAddressingMode(const DataLayout &DL,
                                               const AddrMode &AM, Type *Ty,
                                               unsigned AS,
                                               Instruction *I) const {
+  // No scalable offsets allowed.
+  if (AM.OffsetIsScalable)
+    return false;
+
   // Vector type r+i form is supported since power9 as DQ form. We don't check
   // the offset matching DQ form requirement(off % 16 == 0), because on PowerPC,
   // imm form is preferred and the offset can be adjusted to use imm form later
diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
index dde1882f5eea83..a41738fb718f44 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -1753,6 +1753,10 @@ bool RISCVTargetLowering::isLegalAddressingMode(const DataLayout &DL,
                                                 const AddrMode &AM, Type *Ty,
                                                 unsigned AS,
                                                 Instruction *I) const {
+  // No scalable offsets allowed.
+  if (AM.OffsetIsScalable)
+    return false;
+
   // No global is ever allowed as a base.
   if (AM.BaseGV)
     return false;
diff --git a/llvm/lib/Target/SystemZ/SystemZISelLowering.cpp b/llvm/lib/Target/SystemZ/SystemZISelLowering.cpp
index 3b85a6ac0371ed..5feba75d92f54a 100644
--- a/llvm/lib/Target/SystemZ/SystemZISelLowering.cpp
+++ b/llvm/lib/Target/SystemZ/SystemZISelLowering.cpp
@@ -1058,6 +1058,10 @@ supportedAddressingMode(Instruction *I, bool HasVector) {
 
 bool SystemZTargetLowering::isLegalAddressingMode(const DataLayout &DL,
        const AddrMode &AM, Type *Ty, unsigned AS, Instruction *I) const {
+  // No scalable offsets allowed.
+  if (AM.OffsetIsScalable)
+    return false;
+
   // Punt on globals for now, although they can be used in limited
   // RELATIVE LONG cases.
   if (AM.BaseGV)
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
index 7c47790d1e3515..51425a684a4145 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
@@ -784,6 +784,10 @@ bool WebAssemblyTargetLowering::isLegalAddressingMode(const DataLayout &DL,
                                                       const AddrMode &AM,
                                                       Type *Ty, unsigned AS,
                                                       Instruction *I) const {
+  // No scalable offsets allowed.
+  if (AM.OffsetIsScalable)
+    return false;
+
   // WebAssembly offsets are added as unsigned without wrapping. The
   // isLegalAddressingMode gives us no way to determine if wrapping could be
   // happening, so we approximate this by accepting only non-negative offsets.
diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index 0722c402348ee0..30674cf8deeabe 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -33664,6 +33664,10 @@ bool X86TargetLowering::isLegalAddressingMode(const DataLayout &DL,
                                               const AddrMode &AM, Type *Ty,
                                               unsigned AS,
                                               Instruction *I) const {
+  // No scalable offsets allowed.
+  if (AM.OffsetIsScalable)
+    return false;
+
   // X86 supports extremely general addressing modes.
   CodeModel::Model M = getTargetMachine().getCodeModel();
 
diff --git a/llvm/lib/Target/XCore/XCoreISelLowering.cpp b/llvm/lib/Target/XCore/XCoreISelLowering.cpp
index 18feeaadb03c83..e3518b2707e122 100644
--- a/llvm/lib/Target/XCore/XCoreISelLowering.cpp
+++ b/llvm/lib/Target/XCore/XCoreISelLowering.cpp
@@ -1787,6 +1787,10 @@ bool XCoreTargetLowering::isLegalAddressingMode(const DataLayout &DL,
                                                 const AddrMode &AM, Type *Ty,
                                                 unsigned AS,
                                                 Instruction *I) const {
+  // No scalable offsets allowed.
+  if (AM.OffsetIsScalable)
+    return false;
+
   if (Ty->getTypeID() == Type::VoidTyID)
     return AM.Scale == 0 && isImmUs(AM.BaseOffs) && isImmUs4(AM.BaseOffs);
 
diff --git a/llvm/unittests/Target/AArch64/AddressingModes.cpp b/llvm/unittests/Target/AArch64/AddressingModes.cpp
index 284ea7ae9233ed..0c086b83242600 100644
--- a/llvm/unittests/Target/AArch64/AddressingModes.cpp
+++ b/llvm/unittests/Target/AArch64/AddressingModes.cpp
@@ -13,11 +13,13 @@ using namespace llvm;
 namespace {
 
 struct AddrMode : public TargetLowering::AddrMode {
-  constexpr AddrMode(GlobalValue *GV, int64_t Offs, bool HasBase, int64_t S) {
+  constexpr AddrMode(GlobalValue *GV, int64_t Offs, bool HasBase, int64_t S,
+                     bool ScalableOffset = false) {
     BaseGV = GV;
     BaseOffs = Offs;
     HasBaseReg = HasBase;
     Scale = S;
+    OffsetIsScalable = ScalableOffset;
   }
 };
 struct TestCase {
@@ -153,6 +155,41 @@ const std::initializer_list<TestCase> Tests = {
     {{nullptr, 4096 + 1, true, 0}, 8, false},
 
 };
+
+struct SVETestCase {
+  AddrMode AM;
+  unsigned TypeBits;
+  unsigned NumElts;
+  bool Result;
+};
+
+const std::initializer_list<SVETestCase> SVETests = {
+    // {BaseGV, BaseOffs, HasBaseReg, Scale, Scalable}, EltBits, Count, Result
+    // Test immediate range -- [-8,7] vector's worth.
+    // <vscale x 16 x i8>, increment by one vector
+    {{nullptr, 16, true, 0, true}, 8, 16, true},
+    // <vscale x 4 x i32>, increment by eight vectors
+    {{nullptr, 128, true, 0, true}, 32, 4, false},
+    // <vscale x 8 x i16>, increment by seven vectors
+    {{nullptr, 112, true, 0, true}, 16, 8, true},
+    // <vscale x 2 x i64>, decrement by eight vectors
+    {{nullptr, -128, true, 0, true}, 64, 2, true},
+    // <vscale x 16 x i8>, decrement by nine vectors
+    {{nullptr, -144...
[truncated]

Copy link

github-actions bot commented Feb 28, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Comment on lines 1534 to 1537
// No scalable offsets allowed.
if (AM.OffsetIsScalable)
return false;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't need to go through all the targets with no scalable vectors

@davemgreen
Copy link
Collaborator

This sounds like a nicer approach that adding the new type. Did you consider just having another int64_t ScalableOffset separate from the Offset? It might not really come up very often that an architecture has both at the same time, but it would be a little more general and fit into AddressingModes being a collection of things all added together.

Copy link
Collaborator

@davemgreen davemgreen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I think this looks good, but had a couple of comments/suggestions.

(AM.ScalableOffset % VecNumBytes == 0) && VecNumBytes <= 16 &&
isPowerOf2_64(VecNumBytes)) {
int64_t Idx = AM.ScalableOffset / (int64_t)VecNumBytes;
return Idx >= -8 && Idx <= 7;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think isInt<4>(Idx), but may have the number of bits off.

// legalization by splitting.
uint64_t VecNumBytes = DL.getTypeSizeInBits(Ty).getKnownMinValue() / 8;
if (AM.HasBaseReg && !AM.BaseOffs && AM.ScalableOffset && !AM.Scale &&
(AM.ScalableOffset % VecNumBytes == 0) && VecNumBytes <= 16 &&
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been staring at this. Is it worth checking the size is one that we expect, or does that not come up or get legalized to another type?

For larger than legal types (that are split into 2) I believe they should be legalized to two address modes, the second with Idx+1. It may be best leaving that for the future though, it is up to you.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I check that the minimum number of bytes is <= 16 (so at most one legal SVE vector) and that it's a power-of-two. I would have restricted it to legal types only, but I've seen several loops which extended a smaller type; so a <vscale x 2 x i8> might be just fine as a type here to determine whether an addressing mode is legal, though it will be extended to <vscale x 2 x i64> in the actual register. We just care about the offset in memory, and from my reading of the ISA the #imm, mul vl addressing goes by the in-memory size.

I have indeed ignored larger-than-legal types for this initial work, as I don't have a motivating example where I need to improve addressing for them.

uint64_t VecElemNumBytes =
DL.getTypeSizeInBits(cast<VectorType>(Ty)->getElementType()) / 8;
return AM.HasBaseReg && !AM.BaseOffs &&
return AM.HasBaseReg && !AM.BaseOffs && !AM.ScalableOffset &&
(AM.Scale == 0 || (uint64_t)AM.Scale == VecElemNumBytes);
}

return AM.HasBaseReg && !AM.BaseOffs && !AM.Scale;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could add a check for !AM.ScalableOffset too, in case it is a struct type.

Comment on lines 1533 to 1534
unsigned AS,
Instruction *I) const {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated formatting change spread through the targets

Copy link
Collaborator

@RKSimon RKSimon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How are targets supposed to know when the scalable offset is supposed to be used? Please can you add useful descriptions to the doxygen comments?

Adds a new parameter to the TTI version of the function, along with
a matching field in the struct for TLI.

This extra bool just indicates that the BaseOffset should be treated
as a scalable quantity (meaning that it should be multiplied by
'vscale' to get the real value at runtime).
Given a base register and a scalable offset (multiplied by vscale),
return true if the offset corresponds to the valid range for the
size of the vector type in memory; e.g. `[X0, llvm#1, mul vl]`
Copy link
Collaborator

@davemgreen davemgreen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. This LGTM

@huntergr-arm huntergr-arm merged commit cd768ec into llvm:main Mar 20, 2024
4 checks passed
@huntergr-arm huntergr-arm deleted the is-legal-addressing-mode-extra-param branch March 20, 2024 10:13
chencha3 pushed a commit to chencha3/llvm-project that referenced this pull request Mar 23, 2024
…3255)

Allows us to indicate that an addressing mode featuring a
vscale-relative immediate offset is supported.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants