Skip to content
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

[SPARC][IAS] Add v8plus feature bit #101367

Merged
merged 3 commits into from
Aug 2, 2024
Merged

Conversation

koachan
Copy link
Contributor

@koachan koachan commented Jul 31, 2024

Implement handling for v8plus feature bit to allow the user to switch between V8 and V8+ mode with 32-bit code.
Currently this only sets the appropriate ELF machine type and flags; codegen changes will be done in future patches.

This is done as a prerequisite for -mv8plus flag on clang (#98713).

@llvmbot llvmbot added backend:Sparc mc Machine (object) code labels Jul 31, 2024
@llvmbot
Copy link
Member

llvmbot commented Jul 31, 2024

@llvm/pr-subscribers-llvm-binary-utilities
@llvm/pr-subscribers-mc

@llvm/pr-subscribers-backend-sparc

Author: Koakuma (koachan)

Changes

Implement handling for v8plus feature bit to allow the user to switch between V8 and V8+ mode with 32-bit code.
Currently this only sets the appropriate ELF machine type; codegen changes will be done in future patches.

This is done as a prerequisite for -mv8plus flag on clang (#98713).


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

5 Files Affected:

  • (modified) llvm/lib/Target/Sparc/MCTargetDesc/SparcAsmBackend.cpp (+3-1)
  • (modified) llvm/lib/Target/Sparc/MCTargetDesc/SparcELFObjectWriter.cpp (+12-5)
  • (modified) llvm/lib/Target/Sparc/MCTargetDesc/SparcMCTargetDesc.h (+4-2)
  • (modified) llvm/lib/Target/Sparc/Sparc.td (+3)
  • (modified) llvm/test/MC/Sparc/elf-sparc-machine-type.s (+5-4)
diff --git a/llvm/lib/Target/Sparc/MCTargetDesc/SparcAsmBackend.cpp b/llvm/lib/Target/Sparc/MCTargetDesc/SparcAsmBackend.cpp
index 29282582b82bd..c2f9111375f41 100644
--- a/llvm/lib/Target/Sparc/MCTargetDesc/SparcAsmBackend.cpp
+++ b/llvm/lib/Target/Sparc/MCTargetDesc/SparcAsmBackend.cpp
@@ -132,6 +132,7 @@ namespace {
   class SparcAsmBackend : public MCAsmBackend {
   protected:
     bool Is64Bit;
+    bool IsV8Plus;
     bool HasV9;
 
   public:
@@ -140,6 +141,7 @@ namespace {
                            ? llvm::endianness::little
                            : llvm::endianness::big),
           Is64Bit(STI.getTargetTriple().isArch64Bit()),
+          IsV8Plus(STI.hasFeature(Sparc::FeatureV8Plus)),
           HasV9(STI.hasFeature(Sparc::FeatureV9)) {}
 
     unsigned getNumFixupKinds() const override {
@@ -359,7 +361,7 @@ namespace {
     std::unique_ptr<MCObjectTargetWriter>
     createObjectTargetWriter() const override {
       uint8_t OSABI = MCELFObjectTargetWriter::getOSABI(OSType);
-      return createSparcELFObjectWriter(Is64Bit, HasV9, OSABI);
+      return createSparcELFObjectWriter(Is64Bit, IsV8Plus, HasV9, OSABI);
     }
   };
 
diff --git a/llvm/lib/Target/Sparc/MCTargetDesc/SparcELFObjectWriter.cpp b/llvm/lib/Target/Sparc/MCTargetDesc/SparcELFObjectWriter.cpp
index bfd71af736231..87c914326fdbf 100644
--- a/llvm/lib/Target/Sparc/MCTargetDesc/SparcELFObjectWriter.cpp
+++ b/llvm/lib/Target/Sparc/MCTargetDesc/SparcELFObjectWriter.cpp
@@ -21,11 +21,16 @@ using namespace llvm;
 namespace {
   class SparcELFObjectWriter : public MCELFObjectTargetWriter {
   public:
-    SparcELFObjectWriter(bool Is64Bit, bool HasV9, uint8_t OSABI)
+    SparcELFObjectWriter(bool Is64Bit, bool IsV8Plus, bool HasV9, uint8_t OSABI)
         : MCELFObjectTargetWriter(
               Is64Bit, OSABI,
-              Is64Bit ? ELF::EM_SPARCV9
-                      : (HasV9 ? ELF::EM_SPARC32PLUS : ELF::EM_SPARC),
+              Is64Bit
+                  ? ELF::EM_SPARCV9
+                  // Note that we still need to emit an EM_SPARC32PLUS object
+                  // even when V8+ isn't explicitly requested, if we're
+                  // targeting a V9-capable CPU. This matches GAS behavior upon
+                  // encountering any V9 instructions in its input.
+                  : ((IsV8Plus || HasV9) ? ELF::EM_SPARC32PLUS : ELF::EM_SPARC),
               /*HasRelocationAddend*/ true) {}
 
     ~SparcELFObjectWriter() override = default;
@@ -148,6 +153,8 @@ bool SparcELFObjectWriter::needsRelocateWithSymbol(const MCValue &,
 }
 
 std::unique_ptr<MCObjectTargetWriter>
-llvm::createSparcELFObjectWriter(bool Is64Bit, bool HasV9, uint8_t OSABI) {
-  return std::make_unique<SparcELFObjectWriter>(Is64Bit, HasV9, OSABI);
+llvm::createSparcELFObjectWriter(bool Is64Bit, bool IsV8Plus, bool HasV9,
+                                 uint8_t OSABI) {
+  return std::make_unique<SparcELFObjectWriter>(Is64Bit, IsV8Plus, HasV9,
+                                                OSABI);
 }
diff --git a/llvm/lib/Target/Sparc/MCTargetDesc/SparcMCTargetDesc.h b/llvm/lib/Target/Sparc/MCTargetDesc/SparcMCTargetDesc.h
index 63419663b722c..4d195c5130c48 100644
--- a/llvm/lib/Target/Sparc/MCTargetDesc/SparcMCTargetDesc.h
+++ b/llvm/lib/Target/Sparc/MCTargetDesc/SparcMCTargetDesc.h
@@ -34,8 +34,10 @@ MCCodeEmitter *createSparcMCCodeEmitter(const MCInstrInfo &MCII,
 MCAsmBackend *createSparcAsmBackend(const Target &T, const MCSubtargetInfo &STI,
                                     const MCRegisterInfo &MRI,
                                     const MCTargetOptions &Options);
-std::unique_ptr<MCObjectTargetWriter>
-createSparcELFObjectWriter(bool Is64Bit, bool HasV9, uint8_t OSABI);
+std::unique_ptr<MCObjectTargetWriter> createSparcELFObjectWriter(bool Is64Bit,
+                                                                 bool IsV8Plus,
+                                                                 bool HasV9,
+                                                                 uint8_t OSABI);
 
 // Defines symbolic names for Sparc v9 ASI tag names.
 namespace SparcASITag {
diff --git a/llvm/lib/Target/Sparc/Sparc.td b/llvm/lib/Target/Sparc/Sparc.td
index 65f372f4376b1..8b1122741b661 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 FeatureV8Plus
+  : SubtargetFeature<"v8plus", "IsV8Plus", "true",
+                     "Enable V8+ mode, allowing use of 64-bit V9 instructions in 32-bit code">;
 def FeatureV8Deprecated
   : SubtargetFeature<"deprecated-v8", "UseV8DeprecatedInsts", "true",
                      "Enable deprecated V8 instructions in V9 mode">;
diff --git a/llvm/test/MC/Sparc/elf-sparc-machine-type.s b/llvm/test/MC/Sparc/elf-sparc-machine-type.s
index 630812394560c..a4bd3607e350d 100644
--- a/llvm/test/MC/Sparc/elf-sparc-machine-type.s
+++ b/llvm/test/MC/Sparc/elf-sparc-machine-type.s
@@ -1,11 +1,12 @@
 ## Emit correct machine type depending on triple and cpu options.
 ## - `-triple sparc` emits an object of type EM_SPARC;
-## - `-triple sparc -mcpu=v9` emits EM_SPARC32PLUS; and
+## - `-triple sparc -mcpu=v9` or `-triple sparc -mattr=+v8plus` emits EM_SPARC32PLUS; and
 ## - `-triple sparcv9` emits EM_SPARCV9.
 
-# RUN: llvm-mc -filetype=obj -triple sparc            %s -o - | llvm-readobj -h - | FileCheck --check-prefixes=SPARC       %s
-# RUN: llvm-mc -filetype=obj -triple sparc   -mcpu=v9 %s -o - | llvm-readobj -h - | FileCheck --check-prefixes=SPARC32PLUS %s
-# RUN: llvm-mc -filetype=obj -triple sparcv9          %s -o - | llvm-readobj -h - | FileCheck --check-prefixes=SPARCV9     %s
+# RUN: llvm-mc -filetype=obj -triple sparc                  %s -o - | llvm-readobj -h - | FileCheck --check-prefixes=SPARC       %s
+# RUN: llvm-mc -filetype=obj -triple sparc         -mcpu=v9 %s -o - | llvm-readobj -h - | FileCheck --check-prefixes=SPARC32PLUS %s
+# RUN: llvm-mc -filetype=obj -triple sparc   -mattr=+v8plus %s -o - | llvm-readobj -h - | FileCheck --check-prefixes=SPARC32PLUS %s
+# RUN: llvm-mc -filetype=obj -triple sparcv9                %s -o - | llvm-readobj -h - | FileCheck --check-prefixes=SPARCV9     %s
 
 # SPARC:       Machine: EM_SPARC (0x2)
 # SPARC32PLUS: Machine: EM_SPARC32PLUS (0x12)

// even when V8+ isn't explicitly requested, if we're
// targeting a V9-capable CPU. This matches GAS behavior upon
// encountering any V9 instructions in its input.
: ((IsV8Plus || HasV9) ? ELF::EM_SPARC32PLUS : ELF::EM_SPARC),
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure this is correct? Does sparc-gcc produce EM_SPARC32PLUS binary by default (taking into account that -mcpu=v9 is the default)?
I only have sparc64-gcc on hands.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, I'm pretty sure of that. GCC does not control object type, GAS does, and what it does is autoselecting the object type to emit based on the contents of the asm file given to it, defaulting to EM_SPARC if no V9 instructions are encountered.

For example, a file containing just an empty function void empty() {} becomes a retl; nop in assembly, and since the resulting asm is free from V9 instructions, GAS will emit an EM_SPARC object, even if you specify -mv8plus in GCC.
On the other hand, something like void fadd(std::atomic<int> v) { v++; } will compile down to a cas, and given that cas is a V9 instruction, GAS will emit an EM_SPARC32PLUS object anyway even if you specify -mno-v8plus in GCC.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, and, except for when targeting LEON, where GAS will always emit an EM_SPARC object, even in the presence of cas.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a pretty surprising behavior given that there are assembler options for V8+ and a note:

Use one of the ‘-A’ options to select one of the SPARC architectures explicitly. If you select an architecture explicitly, as reports a fatal error if it encounters an instruction or feature requiring an incompatible or higher level.

The thing that concerns me is that we unconditionally create SPARC32PLUS objects that the linker might reject to link with SPARC objects. Even if it does not complain, the resulting V8+ executable might be rejected by the loader if the OS does not support V8+.

With that said I think we should create V8+ objects only when explicitly requested. Can we do this or the kernel buildsystem relies on gas bumping ISA level? (Does it not pass -mv8plus to the compiler driver?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a pretty surprising behavior given that there are assembler options for V8+ and a note:

Use one of the ‘-A’ options to select one of the SPARC architectures explicitly. If you select an architecture explicitly, as reports a fatal error if it encounters an instruction or feature requiring an incompatible or higher level.

The thing that concerns me is that we unconditionally create SPARC32PLUS objects that the linker might reject to link with SPARC objects. Even if it does not complain, the resulting V8+ executable might be rejected by the loader if the OS does not support V8+.

Mhm. Looking at the output of gcc -v, it does make use of -Av8plus and friends, it's just it doesn't seem to affect the emitted binary.

On the other hand, the wording in the V8+ spec seems to imply that any 32-bit binaries that use any V9 features must be tagged as V8+ anyways, regardless of whether it uses 64-bit registers or not. From page 2-5:

A binary that uses SPARC V9 and UltraSPARC features, referred to as a V8+ binary, must contain an indication of this in its ELF header since it will not execute in a vanilla V8 environment.


With that said I think we should create V8+ objects only when explicitly requested. Can we do this or the kernel buildsystem relies on gas bumping ISA level? (Does it not pass -mv8plus to the compiler driver?)

In any case, though, the kernel does set -mv8plus in its CFLAGS, so I suppose it would be fine too if we create V8+ objects only when requested?

Copy link
Contributor

Choose a reason for hiding this comment

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

A binary that uses SPARC V9 and UltraSPARC features, referred to as a V8+ binary, must contain an indication of this in its ELF header since it will not execute in a vanilla V8 environment.

The V9 and UltraSPARC features are probably those specified in section 2:

Chapter 2, “Differences”, is comprised of definitions and discussions of the
differences between SPARC V8+ and SPARC V8; also, the SPARC V9 and
UltraSPARC™ features, which comprise the V8+ specification.

In any case, though, the kernel does set -mv8plus in its CFLAGS, so I suppose it would be fine too if we create V8+ objects only when requested?

Let's do it then.

Copy link
Contributor

Choose a reason for hiding this comment

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

The specification also requires e_flags to be set to EF_SPARC_32PLUS.
See AVRELFStreamer.cpp for an example of how to do it.

EF_SPARCV9_TSO = 0x0,
EF_SPARCV9_PSO = 0x1,
EF_SPARCV9_RMO = 0x2,
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah I didn't realize this enum doesn't exist.
It will need to be supported in llvm-readobj etc., probably in separate PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mhm, I think it's better to do that in separate PRs, just like with clang.


EFlags |= getEFlagsForFeatureSet(STI);

MCA.setELFHeaderEFlags(EFlags);
Copy link
Contributor

Choose a reason for hiding this comment

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

This interface has been recently moved to ELFObjectWriter, the PR needs rebasing.

koachan added 3 commits August 1, 2024 19:36
Implement handling for `v8plus` feature bit to allow the user to switch
between V8 and V8+ mode with 32-bit code.

This is done as a prerequisite for `-mv8plus` flag on clang (llvm#98713).
@koachan koachan force-pushed the sparc64-v8plus-backend branch from c1068d5 to 3db1a6d Compare August 2, 2024 01:39
@koachan koachan merged commit aca971d into llvm:main Aug 2, 2024
7 checks passed
@koachan koachan deleted the sparc64-v8plus-backend branch August 2, 2024 03:30
banach-space pushed a commit to banach-space/llvm-project that referenced this pull request Aug 7, 2024
Implement handling for `v8plus` feature bit to allow the user to switch
between V8 and V8+ mode with 32-bit code.
Currently this only sets the appropriate ELF machine type and flags;
codegen changes will be done in future patches.

This is done as a prerequisite for `-mv8plus` flag on clang (llvm#98713).
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