Skip to content

ARM: Move ABI enum from TargetMachine to TargetParser #144725

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

Merged
merged 1 commit into from
Jun 23, 2025

Conversation

arsenm
Copy link
Contributor

@arsenm arsenm commented Jun 18, 2025

Consolidate ABI parsing logic in TargetParser where computeDefaultTargetABI is defined, instead of splitting it into the backend. We need the full ABI information computable in RuntimeLibcallsInfo

Copy link
Contributor Author

arsenm commented Jun 18, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@llvmbot
Copy link
Member

llvmbot commented Jun 18, 2025

@llvm/pr-subscribers-backend-arm

Author: Matt Arsenault (arsenm)

Changes

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

6 Files Affected:

  • (modified) llvm/include/llvm/TargetParser/ARMTargetParser.h (+10)
  • (modified) llvm/lib/Target/ARM/ARMSubtarget.cpp (+7-7)
  • (modified) llvm/lib/Target/ARM/ARMTargetMachine.cpp (+8-27)
  • (modified) llvm/lib/Target/ARM/ARMTargetMachine.h (+3-8)
  • (modified) llvm/lib/Target/ARM/ARMTargetObjectFile.cpp (+1-1)
  • (modified) llvm/lib/TargetParser/ARMTargetParser.cpp (+17)
diff --git a/llvm/include/llvm/TargetParser/ARMTargetParser.h b/llvm/include/llvm/TargetParser/ARMTargetParser.h
index 798c578ced938..3ae6c4956656d 100644
--- a/llvm/include/llvm/TargetParser/ARMTargetParser.h
+++ b/llvm/include/llvm/TargetParser/ARMTargetParser.h
@@ -27,6 +27,13 @@ class Triple;
 
 namespace ARM {
 
+enum ARMABI {
+  ARM_ABI_UNKNOWN,
+  ARM_ABI_APCS,
+  ARM_ABI_AAPCS, // ARM EABI
+  ARM_ABI_AAPCS16
+};
+
 // Arch extension modifiers for CPUs.
 // Note that this is not the same as the AArch64 list
 enum ArchExtKind : uint64_t {
@@ -265,6 +272,9 @@ LLVM_ABI unsigned parseArchVersion(StringRef Arch);
 LLVM_ABI void fillValidCPUArchList(SmallVectorImpl<StringRef> &Values);
 LLVM_ABI StringRef computeDefaultTargetABI(const Triple &TT, StringRef CPU);
 
+LLVM_ABI ARMABI computeTargetABI(const Triple &TT, StringRef CPU,
+                                 StringRef ABIName = "");
+
 /// Get the (LLVM) name of the minimum ARM CPU for the arch we are targeting.
 ///
 /// \param Arch the architecture name (e.g., "armv7s"). If it is an empty
diff --git a/llvm/lib/Target/ARM/ARMSubtarget.cpp b/llvm/lib/Target/ARM/ARMSubtarget.cpp
index 91d385a0b5950..2b66ebde2dcec 100644
--- a/llvm/lib/Target/ARM/ARMSubtarget.cpp
+++ b/llvm/lib/Target/ARM/ARMSubtarget.cpp
@@ -323,17 +323,17 @@ void ARMSubtarget::initSubtargetFeatures(StringRef CPU, StringRef FS) {
 bool ARMSubtarget::isTargetHardFloat() const { return TM.isTargetHardFloat(); }
 
 bool ARMSubtarget::isAPCS_ABI() const {
-  assert(TM.TargetABI != ARMBaseTargetMachine::ARM_ABI_UNKNOWN);
-  return TM.TargetABI == ARMBaseTargetMachine::ARM_ABI_APCS;
+  assert(TM.TargetABI != ARM::ARM_ABI_UNKNOWN);
+  return TM.TargetABI == ARM::ARM_ABI_APCS;
 }
 bool ARMSubtarget::isAAPCS_ABI() const {
-  assert(TM.TargetABI != ARMBaseTargetMachine::ARM_ABI_UNKNOWN);
-  return TM.TargetABI == ARMBaseTargetMachine::ARM_ABI_AAPCS ||
-         TM.TargetABI == ARMBaseTargetMachine::ARM_ABI_AAPCS16;
+  assert(TM.TargetABI != ARM::ARM_ABI_UNKNOWN);
+  return TM.TargetABI == ARM::ARM_ABI_AAPCS ||
+         TM.TargetABI == ARM::ARM_ABI_AAPCS16;
 }
 bool ARMSubtarget::isAAPCS16_ABI() const {
-  assert(TM.TargetABI != ARMBaseTargetMachine::ARM_ABI_UNKNOWN);
-  return TM.TargetABI == ARMBaseTargetMachine::ARM_ABI_AAPCS16;
+  assert(TM.TargetABI != ARM::ARM_ABI_UNKNOWN);
+  return TM.TargetABI == ARM::ARM_ABI_AAPCS16;
 }
 
 bool ARMSubtarget::isROPI() const {
diff --git a/llvm/lib/Target/ARM/ARMTargetMachine.cpp b/llvm/lib/Target/ARM/ARMTargetMachine.cpp
index fee77a44e5e80..c66232ef4dc7a 100644
--- a/llvm/lib/Target/ARM/ARMTargetMachine.cpp
+++ b/llvm/lib/Target/ARM/ARMTargetMachine.cpp
@@ -121,29 +121,10 @@ static std::unique_ptr<TargetLoweringObjectFile> createTLOF(const Triple &TT) {
   return std::make_unique<ARMElfTargetObjectFile>();
 }
 
-static ARMBaseTargetMachine::ARMABI
-computeTargetABI(const Triple &TT, StringRef CPU,
-                 const TargetOptions &Options) {
-  StringRef ABIName = Options.MCOptions.getABIName();
-
-  if (ABIName.empty())
-    ABIName = ARM::computeDefaultTargetABI(TT, CPU);
-
-  if (ABIName == "aapcs16")
-    return ARMBaseTargetMachine::ARM_ABI_AAPCS16;
-  else if (ABIName.starts_with("aapcs"))
-    return ARMBaseTargetMachine::ARM_ABI_AAPCS;
-  else if (ABIName.starts_with("apcs"))
-    return ARMBaseTargetMachine::ARM_ABI_APCS;
-
-  llvm_unreachable("Unhandled/unknown ABI Name!");
-  return ARMBaseTargetMachine::ARM_ABI_UNKNOWN;
-}
-
 static std::string computeDataLayout(const Triple &TT, StringRef CPU,
                                      const TargetOptions &Options,
                                      bool isLittle) {
-  auto ABI = computeTargetABI(TT, CPU, Options);
+  auto ABI = ARM::computeTargetABI(TT, CPU, Options.MCOptions.ABIName);
   std::string Ret;
 
   if (isLittle)
@@ -163,19 +144,19 @@ static std::string computeDataLayout(const Triple &TT, StringRef CPU,
   Ret += "-Fi8";
 
   // ABIs other than APCS have 64 bit integers with natural alignment.
-  if (ABI != ARMBaseTargetMachine::ARM_ABI_APCS)
+  if (ABI != ARM::ARM_ABI_APCS)
     Ret += "-i64:64";
 
   // We have 64 bits floats. The APCS ABI requires them to be aligned to 32
   // bits, others to 64 bits. We always try to align to 64 bits.
-  if (ABI == ARMBaseTargetMachine::ARM_ABI_APCS)
+  if (ABI == ARM::ARM_ABI_APCS)
     Ret += "-f64:32:64";
 
   // We have 128 and 64 bit vectors. The APCS ABI aligns them to 32 bits, others
   // to 64. We always ty to give them natural alignment.
-  if (ABI == ARMBaseTargetMachine::ARM_ABI_APCS)
+  if (ABI == ARM::ARM_ABI_APCS)
     Ret += "-v64:32:64-v128:32:128";
-  else if (ABI != ARMBaseTargetMachine::ARM_ABI_AAPCS16)
+  else if (ABI != ARM::ARM_ABI_AAPCS16)
     Ret += "-v128:64:128";
 
   // Try to align aggregates to 32 bits (the default is 64 bits, which has no
@@ -187,9 +168,9 @@ static std::string computeDataLayout(const Triple &TT, StringRef CPU,
 
   // The stack is 128 bit aligned on NaCl, 64 bit aligned on AAPCS and 32 bit
   // aligned everywhere else.
-  if (TT.isOSNaCl() || ABI == ARMBaseTargetMachine::ARM_ABI_AAPCS16)
+  if (TT.isOSNaCl() || ABI == ARM::ARM_ABI_AAPCS16)
     Ret += "-S128";
-  else if (ABI == ARMBaseTargetMachine::ARM_ABI_AAPCS)
+  else if (ABI == ARM::ARM_ABI_AAPCS)
     Ret += "-S64";
   else
     Ret += "-S32";
@@ -226,7 +207,7 @@ ARMBaseTargetMachine::ARMBaseTargetMachine(const Target &T, const Triple &TT,
                                TT, CPU, FS, Options,
                                getEffectiveRelocModel(TT, RM),
                                getEffectiveCodeModel(CM, CodeModel::Small), OL),
-      TargetABI(computeTargetABI(TT, CPU, Options)),
+      TargetABI(ARM::computeTargetABI(TT, CPU, Options.MCOptions.ABIName)),
       TLOF(createTLOF(getTargetTriple())), isLittle(isLittle) {
 
   // Default to triple-appropriate float ABI
diff --git a/llvm/lib/Target/ARM/ARMTargetMachine.h b/llvm/lib/Target/ARM/ARMTargetMachine.h
index 99fd817c81f89..4a2e9f41376f9 100644
--- a/llvm/lib/Target/ARM/ARMTargetMachine.h
+++ b/llvm/lib/Target/ARM/ARMTargetMachine.h
@@ -20,6 +20,7 @@
 #include "llvm/CodeGen/CodeGenTargetMachineImpl.h"
 #include "llvm/Support/CodeGen.h"
 #include "llvm/Target/TargetMachine.h"
+#include "llvm/TargetParser/ARMTargetParser.h"
 #include <memory>
 #include <optional>
 
@@ -27,12 +28,7 @@ namespace llvm {
 
 class ARMBaseTargetMachine : public CodeGenTargetMachineImpl {
 public:
-  enum ARMABI {
-    ARM_ABI_UNKNOWN,
-    ARM_ABI_APCS,
-    ARM_ABI_AAPCS, // ARM EABI
-    ARM_ABI_AAPCS16
-  } TargetABI;
+  ARM::ARMABI TargetABI;
 
 protected:
   std::unique_ptr<TargetLoweringObjectFile> TLOF;
@@ -73,8 +69,7 @@ class ARMBaseTargetMachine : public CodeGenTargetMachineImpl {
            TargetTriple.getEnvironment() == Triple::EABIHF ||
            (TargetTriple.isOSBinFormatMachO() &&
             TargetTriple.getSubArch() == Triple::ARMSubArch_v7em) ||
-           TargetTriple.isOSWindows() ||
-           TargetABI == ARMBaseTargetMachine::ARM_ABI_AAPCS16;
+           TargetTriple.isOSWindows() || TargetABI == ARM::ARM_ABI_AAPCS16;
   }
 
   bool targetSchedulesPostRAScheduling() const override { return true; };
diff --git a/llvm/lib/Target/ARM/ARMTargetObjectFile.cpp b/llvm/lib/Target/ARM/ARMTargetObjectFile.cpp
index a0a400f938482..cf84f1043cc69 100644
--- a/llvm/lib/Target/ARM/ARMTargetObjectFile.cpp
+++ b/llvm/lib/Target/ARM/ARMTargetObjectFile.cpp
@@ -37,7 +37,7 @@ ARMElfTargetObjectFile::ARMElfTargetObjectFile() {
 void ARMElfTargetObjectFile::Initialize(MCContext &Ctx,
                                         const TargetMachine &TM) {
   const ARMBaseTargetMachine &ARM_TM = static_cast<const ARMBaseTargetMachine &>(TM);
-  bool isAAPCS_ABI = ARM_TM.TargetABI == ARMBaseTargetMachine::ARMABI::ARM_ABI_AAPCS;
+  bool isAAPCS_ABI = ARM_TM.TargetABI == ARM::ARMABI::ARM_ABI_AAPCS;
   bool genExecuteOnly =
       ARM_TM.getMCSubtargetInfo()->hasFeature(ARM::FeatureExecuteOnly);
 
diff --git a/llvm/lib/TargetParser/ARMTargetParser.cpp b/llvm/lib/TargetParser/ARMTargetParser.cpp
index a7a895d872668..9ff6521c5d1e8 100644
--- a/llvm/lib/TargetParser/ARMTargetParser.cpp
+++ b/llvm/lib/TargetParser/ARMTargetParser.cpp
@@ -575,6 +575,23 @@ StringRef ARM::computeDefaultTargetABI(const Triple &TT, StringRef CPU) {
   }
 }
 
+ARM::ARMABI ARM::computeTargetABI(const Triple &TT, StringRef CPU,
+                                  StringRef ABIName) {
+  if (ABIName.empty())
+    ABIName = ARM::computeDefaultTargetABI(TT, CPU);
+
+  if (ABIName == "aapcs16")
+    return ARM_ABI_AAPCS16;
+
+  if (ABIName.starts_with("aapcs"))
+    return ARM_ABI_AAPCS;
+
+  if (ABIName.starts_with("apcs"))
+    return ARM_ABI_APCS;
+
+  return ARM_ABI_UNKNOWN;
+}
+
 StringRef ARM::getARMCPUForArch(const llvm::Triple &Triple, StringRef MArch) {
   if (MArch.empty())
     MArch = Triple.getArchName();

@arsenm arsenm marked this pull request as ready for review June 18, 2025 15:23
@arsenm arsenm force-pushed the users/arsenm/arm/move-abi-enum-to-targetparser branch from 9476487 to d6b506b Compare June 19, 2025 01:03
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.

The patch summary is empty and gives no context as to why this is beneficial (either now or in the future when we are looking back at why things changed). Can you update the description to explain the reason a little? Thanks

@arsenm
Copy link
Contributor Author

arsenm commented Jun 23, 2025

Needs combining with #144894 to be useful in RuntimeLibcallsInfo

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, LGTM

@arsenm arsenm merged commit 3892096 into main Jun 23, 2025
7 checks passed
@arsenm arsenm deleted the users/arsenm/arm/move-abi-enum-to-targetparser branch June 23, 2025 06:23
miguelcsx pushed a commit to miguelcsx/llvm-project that referenced this pull request Jun 23, 2025
Consolidate ABI parsing logic in TargetParser where
computeDefaultTargetABI is defined, instead of splitting it into the
backend. We need the full ABI information computable in
RuntimeLibcallsInfo
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.

3 participants