Skip to content

Conversation

arsenm
Copy link
Contributor

@arsenm arsenm commented Sep 8, 2025

Add a 64-bit feature so a subtarget feature check can tell the
pointer size, for future use with HwMode.

This is kind of a hack, but this is closer to what other targets do.
To use HwModes, there needs to be a subtarget feature. Every other
target kludges the module level properties into a subtarget feature
for use here, which requires pre/post processing the subtarget features.
The APIs for this aren't great. I tried doing something different,
closer to what hexagon does, rather than what x86 does to see if it
was any nicer. It almost is, except for some reason we don't have an
API to directly set a bit in the FeatureBitset.

Also the test coverage for the different ABI options isn't great.
e.g. v9 as a feature almost works, except a single test breaks
that uses a sparc32 triple with an explicit v9 feature.

Add a 64-bit feature so a subtarget feature check can tell the
pointer size, for future use with HwMode.

This is kind of a hack, but this is closer to what other targets do.
To use HwModes, there needs to be a subtarget feature. Every other
target kludges the module level properties into a subtarget feature
for use here, which requires pre/post processing the subtarget features.
The APIs for this aren't great. I tried doing something different,
closer to what hexagon does, rather than what x86 does to see if it
was any nicer. It almost is, except for some reason we don't have an
ABI to directly set a bit in the FeatureBitset.

Also the test coverage for the different ABI options isn't great.
e.g. v9 as a feature almost works, except a single test breaks
that uses a sparc32 triple with an explicit v9 feature.
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-sparc

Author: Matt Arsenault (arsenm)

Changes

Add a 64-bit feature so a subtarget feature check can tell the
pointer size, for future use with HwMode.

This is kind of a hack, but this is closer to what other targets do.
To use HwModes, there needs to be a subtarget feature. Every other
target kludges the module level properties into a subtarget feature
for use here, which requires pre/post processing the subtarget features.
The APIs for this aren't great. I tried doing something different,
closer to what hexagon does, rather than what x86 does to see if it
was any nicer. It almost is, except for some reason we don't have an
ABI to directly set a bit in the FeatureBitset.

Also the test coverage for the different ABI options isn't great.
e.g. v9 as a feature almost works, except a single test breaks
that uses a sparc32 triple with an explicit v9 feature.


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

5 Files Affected:

  • (modified) llvm/lib/Target/Sparc/MCTargetDesc/SparcMCTargetDesc.cpp (+10-2)
  • (modified) llvm/lib/Target/Sparc/MCTargetDesc/SparcMCTargetDesc.h (+1)
  • (modified) llvm/lib/Target/Sparc/Sparc.td (+3)
  • (modified) llvm/lib/Target/Sparc/SparcSubtarget.cpp (+8-2)
  • (modified) llvm/lib/Target/Sparc/SparcSubtarget.h (-4)
diff --git a/llvm/lib/Target/Sparc/MCTargetDesc/SparcMCTargetDesc.cpp b/llvm/lib/Target/Sparc/MCTargetDesc/SparcMCTargetDesc.cpp
index fa07578e512b5..9fa60ee5229ba 100644
--- a/llvm/lib/Target/Sparc/MCTargetDesc/SparcMCTargetDesc.cpp
+++ b/llvm/lib/Target/Sparc/MCTargetDesc/SparcMCTargetDesc.cpp
@@ -81,8 +81,16 @@ static MCRegisterInfo *createSparcMCRegisterInfo(const Triple &TT) {
 static MCSubtargetInfo *
 createSparcMCSubtargetInfo(const Triple &TT, StringRef CPU, StringRef FS) {
   if (CPU.empty())
-    CPU = (TT.getArch() == Triple::sparcv9) ? "v9" : "v8";
-  return createSparcMCSubtargetInfoImpl(TT, CPU, /*TuneCPU*/ CPU, FS);
+    CPU = TT.getArch() == Triple::sparcv9 ? "v9" : "v8";
+
+  MCSubtargetInfo *STI =
+      createSparcMCSubtargetInfoImpl(TT, CPU, /*TuneCPU=*/CPU, FS);
+  if (TT.isSPARC64() && !STI->hasFeature(Sparc::Feature64Bit)) {
+    FeatureBitset Features = STI->getFeatureBits();
+    STI->setFeatureBits(Features.set(Sparc::Feature64Bit));
+  }
+
+  return STI;
 }
 
 static MCTargetStreamer *
diff --git a/llvm/lib/Target/Sparc/MCTargetDesc/SparcMCTargetDesc.h b/llvm/lib/Target/Sparc/MCTargetDesc/SparcMCTargetDesc.h
index a7b0538d683b6..b523366e6adaa 100644
--- a/llvm/lib/Target/Sparc/MCTargetDesc/SparcMCTargetDesc.h
+++ b/llvm/lib/Target/Sparc/MCTargetDesc/SparcMCTargetDesc.h
@@ -28,6 +28,7 @@ class MCRegisterInfo;
 class MCSubtargetInfo;
 class MCTargetOptions;
 class Target;
+class Triple;
 
 MCCodeEmitter *createSparcMCCodeEmitter(const MCInstrInfo &MCII,
                                         MCContext &Ctx);
diff --git a/llvm/lib/Target/Sparc/Sparc.td b/llvm/lib/Target/Sparc/Sparc.td
index cee671e349510..7137e5fbff4ff 100644
--- a/llvm/lib/Target/Sparc/Sparc.td
+++ b/llvm/lib/Target/Sparc/Sparc.td
@@ -34,6 +34,9 @@ def FeatureNoFMULS
 def FeatureV9
   : SubtargetFeature<"v9", "IsV9", "true",
                      "Enable SPARC-V9 instructions">;
+def Feature64Bit : SubtargetFeature<"64bit", "Is64Bit", "true",
+                                    "Enable 64-bit mode", [FeatureV9]>;
+
 def FeatureV8Plus
   : SubtargetFeature<"v8plus", "IsV8Plus", "true",
                      "Enable V8+ mode, allowing use of 64-bit V9 instructions in 32-bit code">;
diff --git a/llvm/lib/Target/Sparc/SparcSubtarget.cpp b/llvm/lib/Target/Sparc/SparcSubtarget.cpp
index f2721ead00697..005930834a0c3 100644
--- a/llvm/lib/Target/Sparc/SparcSubtarget.cpp
+++ b/llvm/lib/Target/Sparc/SparcSubtarget.cpp
@@ -28,10 +28,11 @@ void SparcSubtarget::anchor() { }
 
 SparcSubtarget &SparcSubtarget::initializeSubtargetDependencies(
     StringRef CPU, StringRef TuneCPU, StringRef FS) {
+  const Triple &TT = getTargetTriple();
   // Determine default and user specified characteristics
   std::string CPUName = std::string(CPU);
   if (CPUName.empty())
-    CPUName = getTargetTriple().isSPARC64() ? "v9" : "v8";
+    CPUName = TT.isSPARC64() ? "v9" : "v8";
 
   if (TuneCPU.empty())
     TuneCPU = CPUName;
@@ -39,6 +40,12 @@ SparcSubtarget &SparcSubtarget::initializeSubtargetDependencies(
   // Parse features string.
   ParseSubtargetFeatures(CPUName, TuneCPU, FS);
 
+  if (!Is64Bit && TT.isSPARC64()) {
+    FeatureBitset Features = getFeatureBits();
+    setFeatureBits(Features.set(Sparc::Feature64Bit));
+    Is64Bit = true;
+  }
+
   // Popc is a v9-only instruction.
   if (!IsV9)
     UsePopc = false;
@@ -50,7 +57,6 @@ SparcSubtarget::SparcSubtarget(const StringRef &CPU, const StringRef &TuneCPU,
                                const StringRef &FS, const TargetMachine &TM)
     : SparcGenSubtargetInfo(TM.getTargetTriple(), CPU, TuneCPU, FS),
       ReserveRegister(TM.getMCRegisterInfo()->getNumRegs()),
-      Is64Bit(TM.getTargetTriple().isSPARC64()),
       InstrInfo(initializeSubtargetDependencies(CPU, TuneCPU, FS)),
       TLInfo(TM, *this), FrameLowering(*this) {
   TSInfo = std::make_unique<SparcSelectionDAGInfo>();
diff --git a/llvm/lib/Target/Sparc/SparcSubtarget.h b/llvm/lib/Target/Sparc/SparcSubtarget.h
index f98aef012a867..b1decca0a4f07 100644
--- a/llvm/lib/Target/Sparc/SparcSubtarget.h
+++ b/llvm/lib/Target/Sparc/SparcSubtarget.h
@@ -36,8 +36,6 @@ class SparcSubtarget : public SparcGenSubtargetInfo {
 
   virtual void anchor();
 
-  const bool Is64Bit;
-
 #define GET_SUBTARGETINFO_MACRO(ATTRIBUTE, DEFAULT, GETTER)                    \
   bool ATTRIBUTE = DEFAULT;
 #include "SparcGenSubtargetInfo.inc"
@@ -79,8 +77,6 @@ class SparcSubtarget : public SparcGenSubtargetInfo {
                                                   StringRef TuneCPU,
                                                   StringRef FS);
 
-  bool is64Bit() const { return Is64Bit; }
-
   /// The 64-bit ABI uses biased stack and frame pointers, so the stack frame
   /// of the current function is the area from [%sp+BIAS] to [%fp+BIAS].
   int64_t getStackPointerBias() const {

@arsenm arsenm marked this pull request as ready for review September 8, 2025 13:12
@s-barannikov
Copy link
Contributor

we don't have an ABI to directly set a bit in the FeatureBitset.

ABI -> API

Copy link
Member

@jyknight jyknight left a comment

Choose a reason for hiding this comment

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

Seems reasonable.

And yes, a 32-bit sparc target with a v9 cpu is nearly untested -- which is not too surprising since last I knew it was also not fully implemented.

@arsenm arsenm merged commit d219467 into main Sep 8, 2025
13 checks passed
@arsenm arsenm deleted the users/arsenm/sparc/add-64bit-feature branch September 8, 2025 14:44
"Enable SPARC-V9 instructions">;
def Feature64Bit : SubtargetFeature<"64bit", "Is64Bit", "true",
"Enable 64-bit mode", [FeatureV9]>;

Copy link
Contributor

Choose a reason for hiding this comment

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

Is64Bit predicate in SparcInstrInfo.td should be changed to use this feature.

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