[AArch64][GISel] Support SVE with 128-bit min-size for G_LOAD and G_STORE #92130

Him188 · 2024-05-14T14:36:24Z

This patch adds basic support for scalable vector types in load & store instructions for AArch64 with GISel.

Only scalable vector types with a 128-bit base size are supported, e.g. <vscale x 4 x i32>, <vscale x 16 x i8>.

This patch adapted some ideas from a similar abandoned patch #72976.

…TORE This patch adds basic support for scalable vector types in load & store instructions for AArch64 with GISel. Only scalable vector types with a 128-bit base size are supported, e.g. <vscale x 4 x i32>, <vscale x 16 x i8>. This patch adapted some ideas from a similar abandoned patch llvm#72976.

github-actions · 2024-05-14T14:36:42Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

llvmbot · 2024-05-14T14:37:11Z

@llvm/pr-subscribers-backend-aarch64

@llvm/pr-subscribers-llvm-globalisel

Author: Him188 (Him188)

Changes

This patch adds basic support for scalable vector types in load & store instructions for AArch64 with GISel.

Only scalable vector types with a 128-bit base size are supported, e.g. <vscale x 4 x i32>, <vscale x 16 x i8>.

This patch adapted some ideas from a similar abandoned patch #72976.

Full diff: https://github.com/llvm/llvm-project/pull/92130.diff

10 Files Affected:

(modified) llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h (+4-4)
(modified) llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp (+1-1)
(modified) llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp (+1-1)
(modified) llvm/lib/Target/AArch64/AArch64ISelLowering.cpp (+11-3)
(modified) llvm/lib/Target/AArch64/AArch64RegisterBanks.td (+1-1)
(modified) llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp (+53-6)
(modified) llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp (+90-3)
(modified) llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerCombiner.cpp (+3-3)
(modified) llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp (+7-3)
(added) llvm/test/CodeGen/AArch64/GlobalISel/sve-load-store.ll (+50)

diff --git a/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h b/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h
index 4d147bf20c26a..29939d4619400 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h
@@ -652,17 +652,17 @@ bool GIMatchTableExecutor::executeMatchTable(
       MachineMemOperand *MMO =
           *(State.MIs[InsnID]->memoperands_begin() + MMOIdx);
 
-      unsigned Size = MRI.getType(MO.getReg()).getSizeInBits();
+      const auto Size = MRI.getType(MO.getReg()).getSizeInBits();
       if (MatcherOpcode == GIM_CheckMemorySizeEqualToLLT &&
-          MMO->getSizeInBits().getValue() != Size) {
+          MMO->getSizeInBits() != Size) {
         if (handleReject() == RejectAndGiveUp)
           return false;
       } else if (MatcherOpcode == GIM_CheckMemorySizeLessThanLLT &&
-                 MMO->getSizeInBits().getValue() >= Size) {
+                 MMO->getSizeInBits().getValue() >= Size.getKnownMinValue()) {
         if (handleReject() == RejectAndGiveUp)
           return false;
       } else if (MatcherOpcode == GIM_CheckMemorySizeGreaterThanLLT &&
-                 MMO->getSizeInBits().getValue() <= Size)
+                 MMO->getSizeInBits().getValue() <= Size.getKnownMinValue())
         if (handleReject() == RejectAndGiveUp)
           return false;
 
diff --git a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
index 653e7689b5774..141c7ee15fe39 100644
--- a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
@@ -1080,7 +1080,7 @@ bool CombinerHelper::isIndexedLoadStoreLegal(GLoadStore &LdSt) const {
   LLT Ty = MRI.getType(LdSt.getReg(0));
   LLT MemTy = LdSt.getMMO().getMemoryType();
   SmallVector<LegalityQuery::MemDesc, 2> MemDescrs(
-      {{MemTy, MemTy.getSizeInBits(), AtomicOrdering::NotAtomic}});
+      {{MemTy, MemTy.getSizeInBits().getKnownMinValue(), AtomicOrdering::NotAtomic}});
   unsigned IndexedOpc = getIndexedOpc(LdSt.getOpcode());
   SmallVector<LLT> OpTys;
   if (IndexedOpc == TargetOpcode::G_INDEXED_STORE)
diff --git a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
index 6661127162e52..b14a004d5c4ac 100644
--- a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
@@ -1413,7 +1413,7 @@ bool IRTranslator::translateLoad(const User &U, MachineIRBuilder &MIRBuilder) {
 
 bool IRTranslator::translateStore(const User &U, MachineIRBuilder &MIRBuilder) {
   const StoreInst &SI = cast<StoreInst>(U);
-  if (DL->getTypeStoreSize(SI.getValueOperand()->getType()) == 0)
+  if (DL->getTypeStoreSize(SI.getValueOperand()->getType()).isZero())
     return true;
 
   ArrayRef<Register> Vals = getOrCreateVRegs(*SI.getValueOperand());
diff --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
index c1ca78af5cda8..e0be162e10a97 100644
--- a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -26375,12 +26375,20 @@ bool AArch64TargetLowering::shouldLocalize(
   return TargetLoweringBase::shouldLocalize(MI, TTI);
 }
 
+static bool isScalableTySupported(const unsigned Op) {
+  return Op == Instruction::Load || Op == Instruction::Store;
+}
+
 bool AArch64TargetLowering::fallBackToDAGISel(const Instruction &Inst) const {
-  if (Inst.getType()->isScalableTy())
-    return true;
+  const auto ScalableTySupported = isScalableTySupported(Inst.getOpcode());
+
+  // Fallback for scalable vectors
+  if (Inst.getType()->isScalableTy() && !ScalableTySupported) {
+      return true;
+  }
 
   for (unsigned i = 0; i < Inst.getNumOperands(); ++i)
-    if (Inst.getOperand(i)->getType()->isScalableTy())
+    if (Inst.getOperand(i)->getType()->isScalableTy() && !ScalableTySupported)
       return true;
 
   if (const AllocaInst *AI = dyn_cast<AllocaInst>(&Inst)) {
diff --git a/llvm/lib/Target/AArch64/AArch64RegisterBanks.td b/llvm/lib/Target/AArch64/AArch64RegisterBanks.td
index 615ce7d51d9ba..9e2ed356299e2 100644
--- a/llvm/lib/Target/AArch64/AArch64RegisterBanks.td
+++ b/llvm/lib/Target/AArch64/AArch64RegisterBanks.td
@@ -13,7 +13,7 @@
 def GPRRegBank : RegisterBank<"GPR", [XSeqPairsClass]>;
 
 /// Floating Point/Vector Registers: B, H, S, D, Q.
-def FPRRegBank : RegisterBank<"FPR", [QQQQ]>;
+def FPRRegBank : RegisterBank<"FPR", [QQQQ, ZPR]>;
 
 /// Conditional register: NZCV.
 def CCRegBank : RegisterBank<"CC", [CCR]>;
diff --git a/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp b/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
index 61f5bc2464ee5..bc47443c45c8e 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
@@ -901,6 +901,27 @@ static unsigned selectLoadStoreUIOp(unsigned GenericOpc, unsigned RegBankID,
   return GenericOpc;
 }
 
+/// Select the AArch64 opcode for the G_LOAD or G_STORE operation for scalable 
+/// vectors.
+/// \p ElementSize size of the element of the scalable vector
+static unsigned selectLoadStoreSVEOp(const unsigned GenericOpc,
+                                     const unsigned ElementSize) {
+  const bool isStore = GenericOpc == TargetOpcode::G_STORE;
+  
+  switch (ElementSize) {
+    case 8:
+      return isStore ? AArch64::ST1B : AArch64::LD1B;
+    case 16:
+      return isStore ? AArch64::ST1H : AArch64::LD1H;
+    case 32:
+      return isStore ? AArch64::ST1W : AArch64::LD1W;
+    case 64:
+      return isStore ? AArch64::ST1D : AArch64::LD1D;
+  }
+  
+  return GenericOpc;
+}
+
 /// Helper function for selectCopy. Inserts a subregister copy from \p SrcReg
 /// to \p *To.
 ///
@@ -2853,8 +2874,8 @@ bool AArch64InstructionSelector::select(MachineInstr &I) {
       return false;
     }
 
-    uint64_t MemSizeInBytes = LdSt.getMemSize().getValue();
-    unsigned MemSizeInBits = LdSt.getMemSizeInBits().getValue();
+    uint64_t MemSizeInBytes = LdSt.getMemSize().getValue().getKnownMinValue();
+    unsigned MemSizeInBits = LdSt.getMemSizeInBits().getValue().getKnownMinValue();
     AtomicOrdering Order = LdSt.getMMO().getSuccessOrdering();
 
     // Need special instructions for atomics that affect ordering.
@@ -2906,9 +2927,23 @@ bool AArch64InstructionSelector::select(MachineInstr &I) {
     const LLT ValTy = MRI.getType(ValReg);
     const RegisterBank &RB = *RBI.getRegBank(ValReg, MRI, TRI);
 
+#ifndef NDEBUG
+    if (ValTy.isScalableVector()) {
+        assert(STI.hasSVE() 
+             && "Load/Store register operand is scalable vector "
+                "while SVE is not supported by the target");
+        // assert(RB.getID() == AArch64::SVRRegBankID 
+        //        && "Load/Store register operand is scalable vector "
+        //           "while its register bank is not SVR");
+    }
+#endif
+    
     // The code below doesn't support truncating stores, so we need to split it
     // again.
-    if (isa<GStore>(LdSt) && ValTy.getSizeInBits() > MemSizeInBits) {
+    // Truncate only if type is not scalable vector
+    const bool NeedTrunc = !ValTy.isScalableVector() 
+                      && ValTy.getSizeInBits().getFixedValue() > MemSizeInBits;
+    if (isa<GStore>(LdSt) && NeedTrunc) {
       unsigned SubReg;
       LLT MemTy = LdSt.getMMO().getMemoryType();
       auto *RC = getRegClassForTypeOnBank(MemTy, RB);
@@ -2921,7 +2956,7 @@ bool AArch64InstructionSelector::select(MachineInstr &I) {
                       .getReg(0);
       RBI.constrainGenericRegister(Copy, *RC, MRI);
       LdSt.getOperand(0).setReg(Copy);
-    } else if (isa<GLoad>(LdSt) && ValTy.getSizeInBits() > MemSizeInBits) {
+    } else if (isa<GLoad>(LdSt) && NeedTrunc) {
       // If this is an any-extending load from the FPR bank, split it into a regular
       // load + extend.
       if (RB.getID() == AArch64::FPRRegBankID) {
@@ -2951,10 +2986,19 @@ bool AArch64InstructionSelector::select(MachineInstr &I) {
     // instruction with an updated opcode, or a new instruction.
     auto SelectLoadStoreAddressingMode = [&]() -> MachineInstr * {
       bool IsStore = isa<GStore>(I);
-      const unsigned NewOpc =
-          selectLoadStoreUIOp(I.getOpcode(), RB.getID(), MemSizeInBits);
+      unsigned NewOpc;
+      if (ValTy.isScalableVector()) {
+        NewOpc = selectLoadStoreSVEOp(I.getOpcode(), ValTy.getElementType().getSizeInBits());
+      } else {
+        NewOpc = selectLoadStoreUIOp(I.getOpcode(), RB.getID(), MemSizeInBits);
+      }
       if (NewOpc == I.getOpcode())
         return nullptr;
+
+      if (ValTy.isScalableVector()) {
+        // Add the predicate register operand
+        I.addOperand(MachineOperand::CreatePredicate(true));
+      }
       // Check if we can fold anything into the addressing mode.
       auto AddrModeFns =
           selectAddrModeIndexed(I.getOperand(1), MemSizeInBytes);
@@ -2970,6 +3014,9 @@ bool AArch64InstructionSelector::select(MachineInstr &I) {
       Register CurValReg = I.getOperand(0).getReg();
       IsStore ? NewInst.addUse(CurValReg) : NewInst.addDef(CurValReg);
       NewInst.cloneMemRefs(I);
+      if (ValTy.isScalableVector()) {
+        NewInst.add(I.getOperand(1)); // Copy predicate register
+      }
       for (auto &Fn : *AddrModeFns)
         Fn(NewInst);
       I.eraseFromParent();
diff --git a/llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp b/llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp
index d4aac94d24f12..c4f5b75ce959f 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp
@@ -61,6 +61,79 @@ AArch64LegalizerInfo::AArch64LegalizerInfo(const AArch64Subtarget &ST)
   const LLT v2s64 = LLT::fixed_vector(2, 64);
   const LLT v2p0 = LLT::fixed_vector(2, p0);
 
+  // Scalable vector sizes range from 128 to 2048
+  // Note that subtargets may not support the full range.
+  // See [ScalableVecTypes] below.
+  const LLT nxv16s8 = LLT::scalable_vector(16, s8);
+  const LLT nxv32s8 = LLT::scalable_vector(32, s8);
+  const LLT nxv64s8 = LLT::scalable_vector(64, s8);
+  const LLT nxv128s8 = LLT::scalable_vector(128, s8);
+  const LLT nxv256s8 = LLT::scalable_vector(256, s8);
+
+  const LLT nxv8s16 = LLT::scalable_vector(8, s16);
+  const LLT nxv16s16 = LLT::scalable_vector(16, s16);
+  const LLT nxv32s16 = LLT::scalable_vector(32, s16);
+  const LLT nxv64s16 = LLT::scalable_vector(64, s16);
+  const LLT nxv128s16 = LLT::scalable_vector(128, s16);
+
+  const LLT nxv4s32 = LLT::scalable_vector(4, s32); 
+  const LLT nxv8s32 = LLT::scalable_vector(8, s32); 
+  const LLT nxv16s32 = LLT::scalable_vector(16, s32); 
+  const LLT nxv32s32 = LLT::scalable_vector(32, s32);
+  const LLT nxv64s32 = LLT::scalable_vector(64, s32);
+
+  const LLT nxv2s64 = LLT::scalable_vector(2, s64);
+  const LLT nxv4s64 = LLT::scalable_vector(4, s64);
+  const LLT nxv8s64 = LLT::scalable_vector(8, s64);
+  const LLT nxv16s64 = LLT::scalable_vector(16, s64);
+  const LLT nxv32s64 = LLT::scalable_vector(32, s64);
+
+  const LLT nxv2p0 = LLT::scalable_vector(2, p0);
+  const LLT nxv4p0 = LLT::scalable_vector(4, p0);
+  const LLT nxv8p0 = LLT::scalable_vector(8, p0);
+  const LLT nxv16p0 = LLT::scalable_vector(16, p0);
+  const LLT nxv32p0 = LLT::scalable_vector(32, p0);
+
+  const auto ScalableVec128 = {
+    nxv16s8, nxv8s16, nxv4s32, nxv2s64, nxv2p0,
+  };
+  const auto ScalableVec256 = {
+    nxv32s8, nxv16s16, nxv8s32, nxv4s64, nxv4p0,
+  };
+  const auto ScalableVec512 = {
+    nxv64s8, nxv32s16, nxv16s32, nxv8s64, nxv8p0,
+  };
+  const auto ScalableVec1024 = {
+    nxv128s8, nxv64s16, nxv32s32, nxv16s64, nxv16p0,
+  };
+  const auto ScalableVec2048 = {
+    nxv256s8, nxv128s16, nxv64s32, nxv32s64, nxv32p0,
+  };
+
+  /// Scalable vector types supported by the sub target.
+  /// Empty if SVE is not supported.
+  SmallVector<LLT> ScalableVecTypes;
+  
+  if (ST.hasSVE()) {
+    // Add scalable vector types that are supported by the subtarget
+    const auto MinSize = ST.getMinSVEVectorSizeInBits();
+    auto MaxSize = ST.getMaxSVEVectorSizeInBits();
+    if (MaxSize == 0) {
+      // Unknown max size, assume the target supports all sizes.
+      MaxSize = 2048; 
+    }
+    if (MinSize <= 128 && 128 <= MaxSize)
+      ScalableVecTypes.append(ScalableVec128);
+    if (MinSize <= 256 && 256 <= MaxSize)
+      ScalableVecTypes.append(ScalableVec256);
+    if (MinSize <= 512 && 512 <= MaxSize)
+      ScalableVecTypes.append(ScalableVec512);
+    if (MinSize <= 1024 && 1024 <= MaxSize)
+      ScalableVecTypes.append(ScalableVec1024);
+    if (MinSize <= 2048 && 2048 <= MaxSize)
+      ScalableVecTypes.append(ScalableVec2048);
+  }
+
   std::initializer_list<LLT> PackedVectorAllTypeList = {/* Begin 128bit types */
                                                         v16s8, v8s16, v4s32,
                                                         v2s64, v2p0,
@@ -329,6 +402,18 @@ AArch64LegalizerInfo::AArch64LegalizerInfo(const AArch64Subtarget &ST)
     return ValTy.isPointerVector() && ValTy.getAddressSpace() == 0;
   };
 
+  const auto IsSameScalableVecTy = [=](const LegalityQuery &Query) {
+    // Legal if loading a scalable vector type
+    // into a scalable vector register of the exactly same type
+    if (!Query.Types[0].isScalableVector() || Query.Types[1] != p0)
+      return false;
+    if (Query.MMODescrs[0].MemoryTy != Query.Types[0])
+      return false;
+    if (Query.MMODescrs[0].AlignInBits < 128)
+      return false;
+    return is_contained(ScalableVecTypes, Query.Types[0]);
+  };
+
   getActionDefinitionsBuilder(G_LOAD)
       .customIf([=](const LegalityQuery &Query) {
         return HasRCPC3 && Query.Types[0] == s128 &&
@@ -354,6 +439,7 @@ AArch64LegalizerInfo::AArch64LegalizerInfo(const AArch64Subtarget &ST)
       // These extends are also legal
       .legalForTypesWithMemDesc(
           {{s32, p0, s8, 8}, {s32, p0, s16, 8}, {s64, p0, s32, 8}})
+      .legalIf(IsSameScalableVecTy)
       .widenScalarToNextPow2(0, /* MinSize = */ 8)
       .clampMaxNumElements(0, s8, 16)
       .clampMaxNumElements(0, s16, 8)
@@ -398,7 +484,9 @@ AArch64LegalizerInfo::AArch64LegalizerInfo(const AArch64Subtarget &ST)
            {s64, p0, s64, 8},   {s64, p0, s32, 8}, // truncstorei32 from s64
            {p0, p0, s64, 8},    {s128, p0, s128, 8},  {v16s8, p0, s128, 8},
            {v8s8, p0, s64, 8},  {v4s16, p0, s64, 8},  {v8s16, p0, s128, 8},
-           {v2s32, p0, s64, 8}, {v4s32, p0, s128, 8}, {v2s64, p0, s128, 8}})
+           {v2s32, p0, s64, 8}, {v4s32, p0, s128, 8}, {v2s64, p0, s128, 8},
+          })
+      .legalIf(IsSameScalableVecTy)
       .clampScalar(0, s8, s64)
       .lowerIf([=](const LegalityQuery &Query) {
         return Query.Types[0].isScalar() &&
@@ -440,8 +528,7 @@ AArch64LegalizerInfo::AArch64LegalizerInfo(const AArch64Subtarget &ST)
           {p0, v4s32, v4s32, 8},
           {p0, v2s64, v2s64, 8},
           {p0, v2p0, v2p0, 8},
-          {p0, s128, s128, 8},
-      })
+          {p0, s128, s128, 8}})
       .unsupported();
 
   auto IndexedLoadBasicPred = [=](const LegalityQuery &Query) {
diff --git a/llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerCombiner.cpp b/llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerCombiner.cpp
index d8ca5494ba50a..5830489e8ef90 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerCombiner.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerCombiner.cpp
@@ -309,7 +309,7 @@ bool matchSplitStoreZero128(MachineInstr &MI, MachineRegisterInfo &MRI) {
   if (!Store.isSimple())
     return false;
   LLT ValTy = MRI.getType(Store.getValueReg());
-  if (!ValTy.isVector() || ValTy.getSizeInBits() != 128)
+  if (!ValTy.isVector() || ValTy.getSizeInBits().getKnownMinValue() != 128)
     return false;
   if (Store.getMemSizeInBits() != ValTy.getSizeInBits())
     return false; // Don't split truncating stores.
@@ -657,8 +657,8 @@ bool AArch64PostLegalizerCombiner::optimizeConsecutiveMemOpAddressing(
         Register PtrBaseReg;
         APInt Offset;
         LLT StoredValTy = MRI.getType(St->getValueReg());
-        unsigned ValSize = StoredValTy.getSizeInBits();
-        if (ValSize < 32 || St->getMMO().getSizeInBits() != ValSize)
+        const auto ValSize = StoredValTy.getSizeInBits();
+        if (ValSize.getKnownMinValue() < 32 || St->getMMO().getSizeInBits() != ValSize)
           continue;
 
         Register PtrReg = St->getPointerReg();
diff --git a/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp b/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
index 44ba9f0429e67..f249729b4b4ab 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
@@ -257,6 +257,7 @@ AArch64RegisterBankInfo::getRegBankFromRegClass(const TargetRegisterClass &RC,
   case AArch64::QQRegClassID:
   case AArch64::QQQRegClassID:
   case AArch64::QQQQRegClassID:
+  case AArch64::ZPRRegClassID:
     return getRegBank(AArch64::FPRRegBankID);
   case AArch64::GPR32commonRegClassID:
   case AArch64::GPR32RegClassID:
@@ -740,11 +741,14 @@ AArch64RegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
     LLT Ty = MRI.getType(MO.getReg());
     if (!Ty.isValid())
       continue;
-    OpSize[Idx] = Ty.getSizeInBits();
+    OpSize[Idx] = Ty.getSizeInBits().getKnownMinValue();
 
-    // As a top-level guess, vectors go in FPRs, scalars and pointers in GPRs.
+    // As a top-level guess, scalable vectors go in SVRs, non-scalable
+    // vectors go in FPRs, scalars and pointers in GPRs.
     // For floating-point instructions, scalars go in FPRs.
-    if (Ty.isVector() || isPreISelGenericFloatingPointOpcode(Opc) ||
+    if (Ty.isScalableVector()) 
+      OpRegBankIdx[Idx] = PMI_FirstFPR;
+    else if (Ty.isVector() || isPreISelGenericFloatingPointOpcode(Opc) ||
         Ty.getSizeInBits() > 64)
       OpRegBankIdx[Idx] = PMI_FirstFPR;
     else
diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/sve-load-store.ll b/llvm/test/CodeGen/AArch64/GlobalISel/sve-load-store.ll
new file mode 100644
index 0000000000000..7a794387eb011
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/sve-load-store.ll
@@ -0,0 +1,50 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 4
+; RUN: llc -mtriple=aarch64-linux-gnu -mattr=+sve -global-isel < %s | FileCheck %s
+
+define void @scalable_v16i8(ptr noalias nocapture noundef %l0, ptr noalias nocapture noundef %l1) {
+; CHECK-LABEL: scalable_v16i8:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    ptrue p0.b
+; CHECK-NEXT:    ld1b { z0.b }, p0/z, [x0]
+; CHECK-NEXT:    st1b { z0.b }, p0, [x1]
+; CHECK-NEXT:    ret
+  %l3 = load <vscale x 16 x i8>, ptr %l0, align 16
+  store <vscale x 16 x i8> %l3, ptr %l1, align 16
+  ret void
+}
+
+define void @scalable_v8i16(ptr noalias nocapture noundef %l0, ptr noalias nocapture noundef %l1) {
+; CHECK-LABEL: scalable_v8i16:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    ptrue p0.h
+; CHECK-NEXT:    ld1h { z0.h }, p0/z, [x0]
+; CHECK-NEXT:    st1h { z0.h }, p0, [x1]
+; CHECK-NEXT:    ret
+  %l3 = load <vscale x 8 x i16>, ptr %l0, align 16
+  store <vscale x 8 x i16> %l3, ptr %l1, align 16
+  ret void
+}
+
+define void @scalable_v4i32(ptr noalias nocapture noundef %l0, ptr noalias nocapture noundef %l1) {
+; CHECK-LABEL: scalable_v4i32:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    ptrue p0.s
+; CHECK-NEXT:    ld1w { z0.s }, p0/z, [x0]
+; CHECK-NEXT:    st1w { z0.s }, p0, [x1]
+; CHECK-NEXT:    ret
+  %l3 = load <vscale x 4 x i32>, ptr %l0, align 16
+  store <vscale x 4 x i32> %l3, ptr %l1, align 16
+  ret void
+}
+
+define void @scalable_v2i64(ptr noalias nocapture noundef %l0, ptr noalias nocapture noundef %l1) {
+; CHECK-LABEL: scalable_v2i64:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    ptrue p0.d
+; CHECK-NEXT:    ld1d { z0.d }, p0/z, [x0]
+; CHECK-NEXT:    st1d { z0.d }, p0, [x1]
+; CHECK-NEXT:    ret
+  %l3 = load <vscale x 2 x i64>, ptr %l0, align 16
+  store <vscale x 2 x i64> %l3, ptr %l1, align 16
+  ret void
+}

Him188 · 2024-05-14T14:42:31Z

Requesting review from @sjoerdmeijer, @davemgreen, @tschuett, @arsenm

llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp

dc03-work · 2024-05-14T15:10:01Z

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

+  const auto ScalableTySupported = isScalableTySupported(Inst.getOpcode());
+
+  // Fallback for scalable vectors
+  if (Inst.getType()->isScalableTy() && !ScalableTySupported) {


We also need to distinguish between GlobalISel and FastISel here. FastISel also calls this function, and it doesn't support scalable vectors. The patch you linked uses a command line flag to do this, but there is probably a better way than that.

Hi - Please keep the command line option from #72976 at the moment. It will help having a debug option we can use for testing, but not be limited to implement every single SVE optimization before global-isel becomes production ready.

This doesn't need to be dependant on the operations we currently "support", the normal fallback mechanism for unsupported operations should handle that.

Added option aarch64-enable-sve-gisel which is disabled by default, instead of aarch64-disable-sve-gisel, because I found other options are also "enable" like aarch64-enable-logical-imm

llvm/test/CodeGen/AArch64/GlobalISel/sve-load-store.ll

tschuett · 2024-05-14T16:15:36Z

llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp

+  const LLT nxv32s8 = LLT::scalable_vector(32, s8);
+  const LLT nxv64s8 = LLT::scalable_vector(64, s8);
+  const LLT nxv128s8 = LLT::scalable_vector(128, s8);
+  const LLT nxv256s8 = LLT::scalable_vector(256, s8);


This is a scalable vector, which size is an unknown multiple of 256 * 8. Legalizing loads of scalable vectors is unintuitive.

As far as I know, there is no target that supports nxv256s8. The original patch only listed:

const LLT nxv8s16 = LLT::scalable_vector(8, 16); const LLT nxv4s32 = LLT::scalable_vector(4, 32); const LLT nxv2s64 = LLT::scalable_vector(2, 64);

To make it simpler for now, I would remove all these except for base sizes of 128-bit:

nxv8s16, nxv4s32, and nxv2s64 as listed in the original patch

nxv16s8 since SelectionDAG supports it

nxv2p0 since it has the same effective size as nxv2s64.

tschuett · 2024-05-14T16:18:38Z

You cannot use legalForTypesWithMemDesc for legalizing scalable vector loads?

Him188 · 2024-05-14T16:31:21Z

You cannot use legalForTypesWithMemDesc for legalizing scalable vector loads?

I intended to reduce the manual permutation of the scalable vector types. We also need to check whether ST supports such vector sizes.

Since I have already generated ScalableVecTypes based on ST capabilities, I decided to reuse that information. Will it be better if I generate rules for legalForTypesWithMemDesc instead?

tschuett · 2024-05-14T16:48:58Z

llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp

+
+  if (ST.hasSVE()) {
+    // Add scalable vector types that are supported by the subtarget
+    const auto MinSize = ST.getMinSVEVectorSizeInBits();


I thought the point of SVE is that you can compile on one machine and run on another with a different vector widths.

Agreed, checking ST.hasSVE() is enough, at least for sizes of multiple of 128-bit

There is a const bool HasRCPC3 = ST.hasRCPC3(); in the file. You could add a similar:

const bool HasSVE = ST.hasSVE();

arsenm · 2024-05-14T18:29:59Z

llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp

+/// Select the AArch64 opcode for the G_LOAD or G_STORE operation for scalable 
+/// vectors.
+/// \p ElementSize size of the element of the scalable vector
+static unsigned selectLoadStoreSVEOp(const unsigned GenericOpc,


Is this manually selected in the DAG path? Can you do this in tablegen?

llvm-project/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp

Lines 865 to 902 in e2ec951

/// Select the AArch64 opcode for the G_LOAD or G_STORE operation \p GenericOpc,

/// appropriate for the (value) register bank \p RegBankID and of memory access

/// size \p OpSize. This returns the variant with the base+unsigned-immediate

/// addressing mode (e.g., LDRXui).

/// \returns \p GenericOpc if the combination is unsupported.

static unsigned selectLoadStoreUIOp(unsigned GenericOpc, unsigned RegBankID,

unsigned OpSize) {

const bool isStore = GenericOpc == TargetOpcode::G_STORE;

switch (RegBankID) {

case AArch64::GPRRegBankID:

switch (OpSize) {

case 8:

return isStore ? AArch64::STRBBui : AArch64::LDRBBui;

case 16:

return isStore ? AArch64::STRHHui : AArch64::LDRHHui;

case 32:

return isStore ? AArch64::STRWui : AArch64::LDRWui;

case 64:

return isStore ? AArch64::STRXui : AArch64::LDRXui;

}

break;

case AArch64::FPRRegBankID:

switch (OpSize) {

case 8:

return isStore ? AArch64::STRBui : AArch64::LDRBui;

case 16:

return isStore ? AArch64::STRHui : AArch64::LDRHui;

case 32:

return isStore ? AArch64::STRSui : AArch64::LDRSui;

case 64:

return isStore ? AArch64::STRDui : AArch64::LDRDui;

case 128:

return isStore ? AArch64::STRQui : AArch64::LDRQui;

}

break;

}

return GenericOpc;

}

Currently we are already doing selectLoadStoreUIOp manually. I'm following the same idea.
Making them select through TableGen would require more work and can be a separate patch.

I did some investigation and found that the patch is actually using the existing tablegen-ed patterns in selectImpl. The added instruction selector code is just skipped.

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

madhur13490 · 2024-05-15T14:51:42Z

llvm/lib/Target/AArch64/AArch64RegisterBanks.td

@@ -13,7 +13,7 @@
 def GPRRegBank : RegisterBank<"GPR", [XSeqPairsClass]>;

 /// Floating Point/Vector Registers: B, H, S, D, Q.
-def FPRRegBank : RegisterBank<"FPR", [QQQQ]>;
+def FPRRegBank : RegisterBank<"FPR", [QQQQ, ZPR]>;


The comment needs update to include ZPR too.

madhur13490 · 2024-05-15T14:55:24Z

llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp

@@ -1413,7 +1413,7 @@ bool IRTranslator::translateLoad(const User &U, MachineIRBuilder &MIRBuilder) {

 bool IRTranslator::translateStore(const User &U, MachineIRBuilder &MIRBuilder) {
  const StoreInst &SI = cast<StoreInst>(U);
-  if (DL->getTypeStoreSize(SI.getValueOperand()->getType()) == 0)
+  if (DL->getTypeStoreSize(SI.getValueOperand()->getType()).isZero())


This seems unrelated change. Isn't it?

It's needed, otherwise we implicitly cast a ScalableVectorTy into ScalarTy and will fail.

TypeSize::operator TypeSize::ScalarTy() const { if (isScalable()) { reportInvalidSizeRequest( "Cannot implicitly convert a scalable size to a fixed-width size in " "`TypeSize::operator ScalarTy()`"); return getKnownMinValue(); } return getFixedValue(); }

aemerson · 2024-05-17T21:20:39Z

llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp

    // For floating-point instructions, scalars go in FPRs.
-    if (Ty.isVector() || isPreISelGenericFloatingPointOpcode(Opc) ||
+    if (Ty.isScalableVector()) 


Isn't isScalableVector() already covered by isVector()?

tschuett · 2024-05-20T06:48:35Z

llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp

-    if (Ty.isVector() || isPreISelGenericFloatingPointOpcode(Opc) ||
+    if (Ty.isScalableVector()) 
+      OpRegBankIdx[Idx] = PMI_FirstFPR;
+    else if (Ty.isVector() || isPreISelGenericFloatingPointOpcode(Opc) ||


We have to distinguish between scalable and fixed sized vectors explicitly.

Ty.getSizeInBits()

asserts for scalable vectors.

or put all vectors into the first if. The second is for > 64 bits.

… by default

tschuett · 2024-05-21T10:57:55Z

llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp

@@ -329,6 +335,20 @@ AArch64LegalizerInfo::AArch64LegalizerInfo(const AArch64Subtarget &ST)
    return ValTy.isPointerVector() && ValTy.getAddressSpace() == 0;
  };

+  if (ST.hasSVE()) {
+    for (const auto OpCode : {G_LOAD, G_STORE}) {
+      getActionDefinitionsBuilder(OpCode)


IIRC, you cannot do getActionDefinitionsBuilder for the same opcode twice, SVE and non-SVE.

Look for G_ABS.

LegalizeRuleSet &LegalizerInfo::getActionDefinitionsBuilder(unsigned Opcode) { unsigned OpcodeIdx = getActionDefinitionsIdx(Opcode); auto &Result = RulesForOpcode[OpcodeIdx]; assert(!Result.isAliasedByAnother() && "Modifying this opcode will modify aliases"); return Result; }

getActionDefinitionsBuilder just returns a reference so I think doing it twice is not a problem.

auto &LoadActions = getActionDefinitionsBuilder(G_LOAD); if (HasSVE)

Changed accordingly

madhur13490 · 2024-05-21T15:36:26Z

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

+// with some instructions.
+// See [AArch64TargetLowering::fallbackToDAGISel] for implementation details.
+static cl::opt<bool> EnableSVEGISel(
+    "aarch64-enable-sve-gisel", cl::Hidden,


Better would be -aarch64-enable-gisel-sve.

Agreed, we already have aarch64-enable-gisel-ldst-prelegal following the same pattern.

davemgreen · 2024-05-21T10:04:23Z

llvm/test/CodeGen/AArch64/GlobalISel/sve-load-store.ll

+  ret void
+}
+
+define void @scalable_v2i64(ptr %l0, ptr %l1) {


Add a test for ptrs too?

It turns out ptrs do not work after adding the test.
It failed to match an existing tablegen pattern during instruction selection. SDAG seems to handle ptrs as s64 but I'm not sure how GISEL handles ptrs. Does it also convert ptrs to s64 at some stage or need special instruction selection pattern for that?

BTW, in RISCV they don't define nxvNp0 types.

GlobalISel uses p<n> (only p0 is used right now?) throughout the pipeline, afaik its not converted to integers anywhere.

AArch64 only uses p0. It is as pointer in address space 0. If we cannot select nxv2p0, then we should not legalize it.

There are two options for ptrs that are stored

Scalars are copied it to integer in AArch64InstructionSelector::preISelLower

Vectors are bitcast to integer vectors in AArch64LegalizerInfo::legalizeLoadStore, by marking them as custom.
We only really need one method though. The vector stores should be using bitcastIf not customIf and the scalars could hopefully use the same method. This might mean that G_BITCAST needed to be supported for scalable vectors though, if it was using the same method.

I vote for postponing support for nxv2p0. The patch is large enough. Bitcasting seems to be preferable.

Yeah that sounds OK to me. Perhaps add a TODO about scalable vector ptr stores, so we can look into it later.

Removed legalizer rules for nzv2p0. Added a TODO

davemgreen · 2024-05-21T10:07:49Z

llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp

+      getActionDefinitionsBuilder(OpCode)
+      .legalForTypesWithMemDesc({
+        // 128 bit base sizes
+        {nxv16s8, p0, nxv16s8, 128},


Can the Align be lower than 128?

Is it appropriate to allow a minimum of 8? 128 seems to be more efficient but 8 should also work.

Yeah 8 should be fine. SDAG seems to accept the smaller alignments.

Ok, changed to 8

davemgreen · 2024-05-21T19:58:43Z

llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h

@@ -652,17 +652,17 @@ bool GIMatchTableExecutor::executeMatchTable(
      MachineMemOperand *MMO =
          *(State.MIs[InsnID]->memoperands_begin() + MMOIdx);

-      unsigned Size = MRI.getType(MO.getReg()).getSizeInBits();
+      const auto Size = MRI.getType(MO.getReg()).getSizeInBits();


Use TypeSize.

davemgreen · 2024-05-21T20:20:17Z

llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h

        if (handleReject() == RejectAndGiveUp)
          return false;
      } else if (MatcherOpcode == GIM_CheckMemorySizeLessThanLLT &&
-                 MMO->getSizeInBits().getValue() >= Size) {
+                 MMO->getSizeInBits().getValue() >= Size.getKnownMinValue()) {


I believe these should either use TypeSize::isKnownGE or test that the types are equally scalable.

The Size in GIM_CheckMemorySizeEqualTo should maybe be scalable too, although as far as I understand none of that is used at the moment.

davemgreen · 2024-05-21T20:32:27Z

llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp

+    if (ValTy.isScalableVector()) {
+        assert(STI.hasSVE() 
+             && "Load/Store register operand is scalable vector "
+                "while SVE is not supported by the target");
+    }


assert((!ValTy.isScalableVector() || STI.hasSVE()) && "Load/Store register operand is scalable vector while SVE is not " "supported by the target");

Reverted this change because we can reuse tablegen pattern and will not reach here

davemgreen · 2024-05-21T21:28:53Z

llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp

@@ -398,7 +420,8 @@ AArch64LegalizerInfo::AArch64LegalizerInfo(const AArch64Subtarget &ST)
           {s64, p0, s64, 8},   {s64, p0, s32, 8}, // truncstorei32 from s64
           {p0, p0, s64, 8},    {s128, p0, s128, 8},  {v16s8, p0, s128, 8},
           {v8s8, p0, s64, 8},  {v4s16, p0, s64, 8},  {v8s16, p0, s128, 8},
-           {v2s32, p0, s64, 8}, {v4s32, p0, s128, 8}, {v2s64, p0, s128, 8}})
+           {v2s32, p0, s64, 8}, {v4s32, p0, s128, 8}, {v2s64, p0, s128, 8},
+          })


Undo this, unless it is clang-formatted on purpose.

davemgreen · 2024-05-21T21:28:58Z

llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp

@@ -440,8 +463,7 @@ AArch64LegalizerInfo::AArch64LegalizerInfo(const AArch64Subtarget &ST)
          {p0, v4s32, v4s32, 8},
          {p0, v2s64, v2s64, 8},
          {p0, v2p0, v2p0, 8},
-          {p0, s128, s128, 8},
-      })
+          {p0, s128, s128, 8}})


Done, formatted the entire patch.

davemgreen · 2024-05-21T21:29:43Z

llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerCombiner.cpp

@@ -309,7 +309,7 @@ bool matchSplitStoreZero128(MachineInstr &MI, MachineRegisterInfo &MRI) {
  if (!Store.isSimple())
    return false;
  LLT ValTy = MRI.getType(Store.getValueReg());
-  if (!ValTy.isVector() || ValTy.getSizeInBits() != 128)
+  if (!ValTy.isVector() || ValTy.getSizeInBits().getKnownMinValue() != 128)


This should have a check for !Scalable, I believe.

Done, reverted this line and added this above instead.

if (ValTy.isScalableVector()) return false;

davemgreen · 2024-05-21T21:32:04Z

llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerCombiner.cpp

@@ -657,8 +657,8 @@ bool AArch64PostLegalizerCombiner::optimizeConsecutiveMemOpAddressing(
        Register PtrBaseReg;
        APInt Offset;
        LLT StoredValTy = MRI.getType(St->getValueReg());
-        unsigned ValSize = StoredValTy.getSizeInBits();
-        if (ValSize < 32 || St->getMMO().getSizeInBits() != ValSize)
+        const auto ValSize = StoredValTy.getSizeInBits();


I think somewhere up above in this function we should bail out if the vector is scalable.

Done, replaced with a bail-out check for operand 0 being scalable vector.

davemgreen · 2024-05-21T21:57:53Z

llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp

@@ -2853,8 +2874,8 @@ bool AArch64InstructionSelector::select(MachineInstr &I) {
      return false;


Is this code actually used? Or is the tablegen pattern already used? I'm not sure how the predicates work otherwise.

I reverted the entire file AArch64InstructionSelector.cpp because we can reuse tablegen patterns. So I think we don't need to change here.

…blegen

aemerson

Thanks for working on this as a first step. LGTM now.

davemgreen

Lets do it. LGTM

Him188 · 2024-05-29T12:41:17Z

On the failed CI:
By searching FAIL: there is only one match, OpenMP/target_ast_print.cpp.
I checked that for the commit this PR is based on, this test is also failing. Should we ignore this or rebase the PR?

aemerson · 2024-05-29T16:37:11Z

On the failed CI: By searching FAIL: there is only one match, OpenMP/target_ast_print.cpp. I checked that for the commit this PR is based on, this test is also failing. Should we ignore this or rebase the PR?

Yes you can just ignore that.

github-actions · 2024-05-30T08:11:01Z

@Him188 Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested
by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as
the builds can include changes from many authors. It is not uncommon for your
change to be included in a build that fails due to someone else's changes, or
infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself.
This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!

llvmbot added backend:AArch64 llvm:globalisel labels May 14, 2024

madhur13490 requested review from arsenm, aemerson, madhur13490, davemgreen and dc03-work May 14, 2024 14:58

dc03-work reviewed May 14, 2024

View reviewed changes

Him188 added 2 commits May 14, 2024 16:28

Remove unnecessary attributes in tests

135c91f

Remove unnecessary #ifndef macro around assertions

e2ec951

tschuett reviewed May 14, 2024

View reviewed changes

arsenm reviewed May 14, 2024

View reviewed changes

madhur13490 reviewed May 15, 2024

View reviewed changes

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp Show resolved Hide resolved

madhur13490 reviewed May 15, 2024

View reviewed changes

aemerson reviewed May 17, 2024

View reviewed changes

tschuett reviewed May 20, 2024

View reviewed changes

Him188 added 4 commits May 20, 2024 13:45

Legal only for size of multiple of 128

70b3f21

Update comments on FPRRegBank in AArch64RegisterBanks.td

73a618b

Add option aarch64-enable-sve-gisel to allow SVE in GISel, disabled…

b19d1b4

… by default

Explicitly assign scalable and non-scalable vectors into FPR

fb48ea1

Him188 requested a review from tschuett May 21, 2024 10:37

tschuett reviewed May 21, 2024

View reviewed changes

Use getActionDefinitionsBuilder only once

9dedf0e

Him188 requested review from aemerson, arsenm and madhur13490 May 21, 2024 13:01

madhur13490 reviewed May 21, 2024

View reviewed changes

davemgreen reviewed May 21, 2024

View reviewed changes

Him188 added 13 commits May 22, 2024 09:37

Replace TypeSize usages

ad331f9

Simplify assertion

0c3cce3

Reformat code

4503074

Remove brackets from single statements

22de2ad

Revert formatting change

3be060c

Skip SplitStoreZero128 for scalable vectors

b5c72d7

Skip optimizeConsecutiveMemOpAddressing for scalable vectors

2518b20

Rename option to aarch64-enable-gisel-sve

5a288d8

Unfold hasSVE loop for legalizer

f1a4d7b

Revert instruction selector changes as they are already covered by ta…

09570db

…blegen

Allow alignment of 8

551fec4

Remove legal rules for nxv2p0

5995c15

Add TODO for nxv2p0

e53b252

aemerson approved these changes May 28, 2024

View reviewed changes

davemgreen approved these changes May 29, 2024

View reviewed changes

Him188 merged commit 8bce40b into llvm:main May 30, 2024
4 of 7 checks passed

Him188 deleted the tguan/sve-load-store-squashed branch May 30, 2024 08:10

Him188 mentioned this pull request Jun 13, 2024

[AArch64][GISel] Translate legal SVE formal arguments and select COPY for SVE #95236

Merged

	/// Select the AArch64 opcode for the G_LOAD or G_STORE operation \p GenericOpc,
	/// appropriate for the (value) register bank \p RegBankID and of memory access
	/// size \p OpSize. This returns the variant with the base+unsigned-immediate
	/// addressing mode (e.g., LDRXui).
	/// \returns \p GenericOpc if the combination is unsupported.
	static unsigned selectLoadStoreUIOp(unsigned GenericOpc, unsigned RegBankID,
	unsigned OpSize) {
	const bool isStore = GenericOpc == TargetOpcode::G_STORE;
	switch (RegBankID) {
	case AArch64::GPRRegBankID:
	switch (OpSize) {
	case 8:
	return isStore ? AArch64::STRBBui : AArch64::LDRBBui;
	case 16:
	return isStore ? AArch64::STRHHui : AArch64::LDRHHui;
	case 32:
	return isStore ? AArch64::STRWui : AArch64::LDRWui;
	case 64:
	return isStore ? AArch64::STRXui : AArch64::LDRXui;
	}
	break;
	case AArch64::FPRRegBankID:
	switch (OpSize) {
	case 8:
	return isStore ? AArch64::STRBui : AArch64::LDRBui;
	case 16:
	return isStore ? AArch64::STRHui : AArch64::LDRHui;
	case 32:
	return isStore ? AArch64::STRSui : AArch64::LDRSui;
	case 64:
	return isStore ? AArch64::STRDui : AArch64::LDRDui;
	case 128:
	return isStore ? AArch64::STRQui : AArch64::LDRQui;
	}
	break;
	}
	return GenericOpc;
	}

		@@ -2853,8 +2874,8 @@ bool AArch64InstructionSelector::select(MachineInstr &I) {
		return false;

[AArch64][GISel] Support SVE with 128-bit min-size for G_LOAD and G_STORE #92130

[AArch64][GISel] Support SVE with 128-bit min-size for G_LOAD and G_STORE #92130

Conversation

Him188 commented May 14, 2024

github-actions bot commented May 14, 2024

llvmbot commented May 14, 2024 • edited Loading

Him188 commented May 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tschuett commented May 14, 2024

Him188 commented May 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Him188 May 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Him188 May 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aemerson left a comment

Choose a reason for hiding this comment

davemgreen left a comment

Choose a reason for hiding this comment

Him188 commented May 29, 2024

aemerson commented May 29, 2024

github-actions bot commented May 30, 2024

llvmbot commented May 14, 2024 •

edited

Loading

Him188 May 21, 2024 •

edited

Loading

Him188 May 28, 2024 •

edited

Loading