Skip to content

Conversation

arsenm
Copy link
Contributor

@arsenm arsenm commented Sep 8, 2025

Just directly check x86_64. isArch64Bit just adds extra
steps around this.

@arsenm arsenm added the backend:X86 label Sep 8, 2025 — with Graphite App
Copy link
Contributor Author

arsenm commented Sep 8, 2025

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

@llvmbot
Copy link
Member

llvmbot commented Sep 8, 2025

@llvm/pr-subscribers-backend-x86

Author: Matt Arsenault (arsenm)

Changes

Just directly check x86_64. isArch64Bit just adds extra
steps around this.


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

5 Files Affected:

  • (modified) llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp (+1-1)
  • (modified) llvm/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.cpp (+1-1)
  • (modified) llvm/lib/Target/X86/X86AsmPrinter.cpp (+4-3)
  • (modified) llvm/lib/Target/X86/X86RegisterInfo.cpp (+6-5)
  • (modified) llvm/lib/Target/X86/X86TargetMachine.cpp (+6-5)
diff --git a/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp b/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
index 865fc0ce8101b..f01805919b9bc 100644
--- a/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
+++ b/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
@@ -1286,7 +1286,7 @@ class DarwinX86AsmBackend : public X86AsmBackend {
   DarwinX86AsmBackend(const Target &T, const MCRegisterInfo &MRI,
                       const MCSubtargetInfo &STI)
       : X86AsmBackend(T, STI), MRI(MRI), TT(STI.getTargetTriple()),
-        Is64Bit(TT.isArch64Bit()) {
+        Is64Bit(TT.getArch() == Triple::x86_64) {
     memset(SavedRegs, 0, sizeof(SavedRegs));
     OffsetSize = Is64Bit ? 8 : 4;
     MoveInstrSize = Is64Bit ? 3 : 2;
diff --git a/llvm/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.cpp b/llvm/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.cpp
index bb1e716c33ed5..b663e57b3c759 100644
--- a/llvm/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.cpp
+++ b/llvm/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.cpp
@@ -48,7 +48,7 @@ std::string X86_MC::ParseX86Triple(const Triple &TT) {
   std::string FS;
   // SSE2 should default to enabled in 64-bit mode, but can be turned off
   // explicitly.
-  if (TT.isArch64Bit())
+  if (TT.getArch() == Triple::x86_64)
     FS = "+64bit-mode,-32bit-mode,-16bit-mode,+sse2";
   else if (TT.getEnvironment() != Triple::CODE16)
     FS = "-64bit-mode,+32bit-mode,-16bit-mode";
diff --git a/llvm/lib/Target/X86/X86AsmPrinter.cpp b/llvm/lib/Target/X86/X86AsmPrinter.cpp
index d406277e440bb..5ea3ed062f363 100644
--- a/llvm/lib/Target/X86/X86AsmPrinter.cpp
+++ b/llvm/lib/Target/X86/X86AsmPrinter.cpp
@@ -194,7 +194,7 @@ void X86AsmPrinter::emitKCFITypeId(const MachineFunction &MF) {
   if (F.getParent()->getModuleFlag("kcfi-arity")) {
     // The ArityToRegMap assumes the 64-bit SysV ABI.
     [[maybe_unused]] const auto &Triple = MF.getTarget().getTargetTriple();
-    assert(Triple.isArch64Bit() && !Triple.isOSWindows());
+    assert(Triple.getArch() == Triple::x86_64 && !Triple.isOSWindows());
 
     // Determine the function's arity (i.e., the number of arguments) at the ABI
     // level by counting the number of parameters that are passed
@@ -896,7 +896,7 @@ void X86AsmPrinter::emitStartOfAsmFile(Module &M) {
 
     if (FeatureFlagsAnd) {
       // Emit a .note.gnu.property section with the flags.
-      assert((TT.isArch32Bit() || TT.isArch64Bit()) &&
+      assert((TT.isArch32Bit() || TT.getArch() == Triple::x86_64) &&
              "CFProtection used on invalid architecture!");
       MCSection *Cur = OutStreamer->getCurrentSectionOnly();
       MCSection *Nt = MMI->getContext().getELFSection(
@@ -904,7 +904,8 @@ void X86AsmPrinter::emitStartOfAsmFile(Module &M) {
       OutStreamer->switchSection(Nt);
 
       // Emitting note header.
-      const int WordSize = TT.isArch64Bit() && !TT.isX32() ? 8 : 4;
+      const int WordSize =
+          TT.getArch() == Triple::x86_64 && !TT.isX32() ? 8 : 4;
       emitAlignment(WordSize == 4 ? Align(4) : Align(8));
       OutStreamer->emitIntValue(4, 4 /*size*/); // data size for "GNU\0"
       OutStreamer->emitIntValue(8 + WordSize, 4 /*size*/); // Elf_Prop size
diff --git a/llvm/lib/Target/X86/X86RegisterInfo.cpp b/llvm/lib/Target/X86/X86RegisterInfo.cpp
index b79e508df7c97..5c3a67d90a994 100644
--- a/llvm/lib/Target/X86/X86RegisterInfo.cpp
+++ b/llvm/lib/Target/X86/X86RegisterInfo.cpp
@@ -53,14 +53,15 @@ static cl::opt<bool>
 extern cl::opt<bool> X86EnableAPXForRelocation;
 
 X86RegisterInfo::X86RegisterInfo(const Triple &TT)
-    : X86GenRegisterInfo((TT.isArch64Bit() ? X86::RIP : X86::EIP),
-                         X86_MC::getDwarfRegFlavour(TT, false),
-                         X86_MC::getDwarfRegFlavour(TT, true),
-                         (TT.isArch64Bit() ? X86::RIP : X86::EIP)) {
+    : X86GenRegisterInfo(
+          (TT.getArch() == Triple::x86_64 ? X86::RIP : X86::EIP),
+          X86_MC::getDwarfRegFlavour(TT, false),
+          X86_MC::getDwarfRegFlavour(TT, true),
+          (TT.getArch() == Triple::x86_64 ? X86::RIP : X86::EIP)) {
   X86_MC::initLLVMToSEHAndCVRegMapping(this);
 
   // Cache some information.
-  Is64Bit = TT.isArch64Bit();
+  Is64Bit = TT.getArch() == Triple::x86_64;
   IsWin64 = Is64Bit && TT.isOSWindows();
   IsUEFI64 = Is64Bit && TT.isUEFI();
 
diff --git a/llvm/lib/Target/X86/X86TargetMachine.cpp b/llvm/lib/Target/X86/X86TargetMachine.cpp
index 6d9c6cdedd9e5..5f76e2a550f40 100644
--- a/llvm/lib/Target/X86/X86TargetMachine.cpp
+++ b/llvm/lib/Target/X86/X86TargetMachine.cpp
@@ -131,7 +131,7 @@ static std::string computeDataLayout(const Triple &TT) {
 
   Ret += DataLayout::getManglingComponent(TT);
   // X86 and x32 have 32 bit pointers.
-  if (!TT.isArch64Bit() || TT.isX32())
+  if (TT.getArch() != Triple::x86_64 || TT.isX32())
     Ret += "-p:32:32";
 
   // Address spaces for 32 bit signed, 32 bit unsigned, and 64 bit pointers.
@@ -140,7 +140,7 @@ static std::string computeDataLayout(const Triple &TT) {
   // Some ABIs align 64 bit integers and doubles to 64 bits, others to 32.
   // 128 bit integers are not specified in the 32-bit ABIs but are used
   // internally for lowering f128, so we match the alignment to that.
-  if (TT.isArch64Bit() || TT.isOSWindows())
+  if (TT.getArch() == Triple::x86_64 || TT.isOSWindows())
     Ret += "-i64:64-i128:128";
   else if (TT.isOSIAMCU())
     Ret += "-i64:32-f64:32";
@@ -150,7 +150,8 @@ static std::string computeDataLayout(const Triple &TT) {
   // Some ABIs align long double to 128 bits, others to 32.
   if (TT.isOSIAMCU())
     ; // No f80
-  else if (TT.isArch64Bit() || TT.isOSDarwin() || TT.isWindowsMSVCEnvironment())
+  else if (TT.getArch() == Triple::x86_64 || TT.isOSDarwin() ||
+           TT.isWindowsMSVCEnvironment())
     Ret += "-f80:128";
   else
     Ret += "-f80:32";
@@ -159,13 +160,13 @@ static std::string computeDataLayout(const Triple &TT) {
     Ret += "-f128:32";
 
   // The registers can hold 8, 16, 32 or, in x86-64, 64 bits.
-  if (TT.isArch64Bit())
+  if (TT.getArch() == Triple::x86_64)
     Ret += "-n8:16:32:64";
   else
     Ret += "-n8:16:32";
 
   // The stack is aligned to 32 bits on some ABIs and 128 bits on others.
-  if ((!TT.isArch64Bit() && TT.isOSWindows()) || TT.isOSIAMCU())
+  if ((TT.getArch() != Triple::x86_64 && TT.isOSWindows()) || TT.isOSIAMCU())
     Ret += "-a:0:32-S32";
   else
     Ret += "-S128";

@arsenm arsenm marked this pull request as ready for review September 8, 2025 09:53
@phoebewang
Copy link
Contributor

We have x86_64 analogues in downstream code. It will make code complicated to check a couple of them each time.

@arsenm
Copy link
Contributor Author

arsenm commented Sep 9, 2025

We have x86_64 analogues in downstream code. It will make code complicated to check a couple of them each time.

Then there should be an isX86_64 helper in Triple like other targets do

@RKSimon
Copy link
Collaborator

RKSimon commented Sep 11, 2025

We have x86_64 analogues in downstream code. It will make code complicated to check a couple of them each time.

Then there should be an isX86_64 helper in Triple like other targets do

Are you intending to handle this or shall one of us?

@arsenm arsenm force-pushed the users/arsenm/x86/simplify-64bit-triple-checks branch from 885e8e6 to 82cedd6 Compare September 11, 2025 12:36
bool isX86_32() const { return getArch() == Triple::x86; }

/// Tests whether the target is x86 (64-bit).
bool isX86_64() const { return getArch() == Triple::x86_64; }
Copy link
Collaborator

Choose a reason for hiding this comment

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

@phoebewang can your downstream targets modify and use this OK?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I think it's OK. Thanks!

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.

LGTM (after merge conflict fixes)

Just directly check x86_64. isArch64Bit just adds extra
steps around this.
@arsenm arsenm force-pushed the users/arsenm/x86/simplify-64bit-triple-checks branch from 82cedd6 to 8bf1253 Compare September 19, 2025 11:45
@arsenm arsenm enabled auto-merge (squash) September 19, 2025 11:46
@arsenm arsenm merged commit cc680fc into main Sep 19, 2025
9 checks passed
@arsenm arsenm deleted the users/arsenm/x86/simplify-64bit-triple-checks branch September 19, 2025 12:49
SeongjaeP pushed a commit to SeongjaeP/llvm-project that referenced this pull request Sep 23, 2025
Just directly check x86_64. isArch64Bit just adds extra
steps around this.
YixingZhang007 pushed a commit to YixingZhang007/llvm-project that referenced this pull request Sep 27, 2025
Just directly check x86_64. isArch64Bit just adds extra
steps around this.
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.

4 participants