Skip to content

Conversation

@arsenm
Copy link
Contributor

@arsenm arsenm commented Nov 26, 2025

Mips seems kind of broken with these options. n32 seems to
override the 64-bit arch with 32-bit pointers, and trying
to use any 32-bit mips triple also just errors with any
options.

Copy link
Contributor Author

arsenm commented Nov 26, 2025

@arsenm arsenm marked this pull request as ready for review November 26, 2025 04:16
@llvmbot llvmbot added the llvm:mc Machine (object) code label Nov 26, 2025
@llvmbot
Copy link
Member

llvmbot commented Nov 26, 2025

@llvm/pr-subscribers-llvm-mc

@llvm/pr-subscribers-backend-mips

Author: Matt Arsenault (arsenm)

Changes

Mips seems kind of broken with these options. n32 seems to
override the 64-bit arch with 32-bit pointers, and trying
to use any 32-bit mips triple also just errors with any
options.


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

4 Files Affected:

  • (modified) llvm/include/llvm/MC/MCTargetOptionsCommandFlags.h (+3-2)
  • (modified) llvm/lib/MC/MCTargetOptionsCommandFlags.cpp (+9-2)
  • (added) llvm/test/tools/opt/infer-data-layout-target-abi.ll (+8)
  • (modified) llvm/tools/opt/optdriver.cpp (+5-4)
diff --git a/llvm/include/llvm/MC/MCTargetOptionsCommandFlags.h b/llvm/include/llvm/MC/MCTargetOptionsCommandFlags.h
index adfdccdb5ab77..168131b43cca8 100644
--- a/llvm/include/llvm/MC/MCTargetOptionsCommandFlags.h
+++ b/llvm/include/llvm/MC/MCTargetOptionsCommandFlags.h
@@ -22,6 +22,7 @@ namespace llvm {
 
 class MCTargetOptions;
 enum class EmitDwarfUnwindType;
+class StringRef;
 
 namespace mc {
 
@@ -62,9 +63,9 @@ LLVM_ABI bool getX86RelaxRelocations();
 
 LLVM_ABI bool getX86Sse2Avx();
 
-LLVM_ABI std::string getABIName();
+LLVM_ABI StringRef getABIName();
 
-LLVM_ABI std::string getAsSecureLogFile();
+LLVM_ABI StringRef getAsSecureLogFile();
 
 /// Create this object with static storage to register mc-related command
 /// line options.
diff --git a/llvm/lib/MC/MCTargetOptionsCommandFlags.cpp b/llvm/lib/MC/MCTargetOptionsCommandFlags.cpp
index ff95ff78fd53a..460667b9c7c08 100644
--- a/llvm/lib/MC/MCTargetOptionsCommandFlags.cpp
+++ b/llvm/lib/MC/MCTargetOptionsCommandFlags.cpp
@@ -24,6 +24,13 @@ using namespace llvm;
     return *NAME##View;                                                        \
   }
 
+#define MCSTROPT(TY, NAME)                                                     \
+  static cl::opt<std::string> *NAME##View;                                     \
+  StringRef llvm::mc::get##NAME() {                                            \
+    assert(NAME##View && "RegisterMCTargetOptionsFlags not created.");         \
+    return *NAME##View;                                                        \
+  }
+
 #define MCOPT_EXP(TY, NAME)                                                    \
   MCOPT(TY, NAME)                                                              \
   std::optional<TY> llvm::mc::getExplicit##NAME() {                            \
@@ -52,8 +59,8 @@ MCOPT(bool, Crel)
 MCOPT(bool, ImplicitMapSyms)
 MCOPT(bool, X86RelaxRelocations)
 MCOPT(bool, X86Sse2Avx)
-MCOPT(std::string, ABIName)
-MCOPT(std::string, AsSecureLogFile)
+MCSTROPT(std::string, ABIName)
+MCSTROPT(std::string, AsSecureLogFile)
 
 llvm::mc::RegisterMCTargetOptionsFlags::RegisterMCTargetOptionsFlags() {
 #define MCBINDOPT(NAME)                                                        \
diff --git a/llvm/test/tools/opt/infer-data-layout-target-abi.ll b/llvm/test/tools/opt/infer-data-layout-target-abi.ll
new file mode 100644
index 0000000000000..4a1ec9305ab6a
--- /dev/null
+++ b/llvm/test/tools/opt/infer-data-layout-target-abi.ll
@@ -0,0 +1,8 @@
+;; Check that we infer the correct datalayout from a target triple
+; RUN: opt -mtriple=mips64-- -S -passes=no-op-module -target-abi=n32 < %s | FileCheck -check-prefix=N32 %s
+; RUN: opt -mtriple=mips64-- -S -passes=no-op-module -target-abi=n64 < %s | FileCheck -check-prefix=N64 %s
+
+target datalayout = ""
+
+; N32: target datalayout = "E-m:e-p:32:32-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+; N64: target datalayout = "E-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
diff --git a/llvm/tools/opt/optdriver.cpp b/llvm/tools/opt/optdriver.cpp
index 63c47151389b5..f8be9f16aada6 100644
--- a/llvm/tools/opt/optdriver.cpp
+++ b/llvm/tools/opt/optdriver.cpp
@@ -37,6 +37,7 @@
 #include "llvm/InitializePasses.h"
 #include "llvm/LinkAllIR.h"
 #include "llvm/LinkAllPasses.h"
+#include "llvm/MC/MCTargetOptionsCommandFlags.h"
 #include "llvm/MC/TargetRegistry.h"
 #include "llvm/Passes/PassPlugin.h"
 #include "llvm/Remarks/HotnessThresholdParser.h"
@@ -516,6 +517,8 @@ optMain(int argc, char **argv,
 
   codegen::MaybeEnableStatistics();
 
+  StringRef ABIName = mc::getABIName(); // FIXME: Handle module flag.
+
   // Load the input module...
   auto SetDataLayout = [&](StringRef IRTriple,
                            StringRef IRLayout) -> std::optional<std::string> {
@@ -541,7 +544,7 @@ optMain(int argc, char **argv,
 
     Triple TT(TripleStr);
 
-    std::string Str = TT.computeDataLayout();
+    std::string Str = TT.computeDataLayout(ABIName);
     if (Str.empty()) {
       errs() << argv[0]
              << ": warning: failed to infer data layout from target triple\n";
@@ -677,9 +680,7 @@ optMain(int argc, char **argv,
 
   RTLIB::RuntimeLibcallsInfo RTLCI(ModuleTriple, codegen::getExceptionModel(),
                                    codegen::getFloatABIForCalls(),
-                                   codegen::getEABIVersion(),
-                                   "", // FIXME: Get ABI name from MCOptions
-                                   VecLib);
+                                   codegen::getEABIVersion(), ABIName, VecLib);
 
   // The -disable-simplify-libcalls flag actually disables all builtin optzns.
   if (DisableSimplifyLibCalls)

Base automatically changed from users/arsenm/opt/stop-creating-tm-compute-datalayout to main November 26, 2025 04:40
return *NAME##View; \
}

#define MCSTROPT(TY, NAME) \
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#define MCSTROPT(TY, NAME) \
#define MCSTROPT(NAME) \

Mips seems kind of broken with these options. n32 seems to
override the 64-bit arch with 32-bit pointers, and trying
to use any 32-bit mips triple also just errors with any
options.
@arsenm arsenm force-pushed the users/arsenm/opt/handle-target-abi-flag branch from 7f4794d to ad30e17 Compare November 26, 2025 14:19
@arsenm arsenm enabled auto-merge (squash) November 26, 2025 14:22
@arsenm arsenm merged commit ff0c347 into main Nov 26, 2025
9 of 10 checks passed
@arsenm arsenm deleted the users/arsenm/opt/handle-target-abi-flag branch November 26, 2025 14:51
tanji-dg pushed a commit to tanji-dg/llvm-project that referenced this pull request Nov 27, 2025
Mips seems kind of broken with these options. n32 seems to
override the 64-bit arch with 32-bit pointers, and trying
to use any 32-bit mips triple also just errors with any
options.
GeneraluseAI pushed a commit to GeneraluseAI/llvm-project that referenced this pull request Nov 27, 2025
Mips seems kind of broken with these options. n32 seems to
override the 64-bit arch with 32-bit pointers, and trying
to use any 32-bit mips triple also just errors with any
options.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants