Address review comments

Author: epilk

llvm/include/llvm/MC/MCAssembler.h

@@ -154,13 +154,12 @@ class MCAssembler {
   bool SubsectionsViaSymbols : 1;
   bool IncrementalLinkerCompatible : 1;
 
-  /// ELF specific e_header and abi version flags
-  // It would be good if there were an MCELFAssembler class to hold these. ELF
-  // header flags are used both by the integrated and standalone assemblers.
+  /// ELF specific e_header flags
+  // It would be good if there were an MCELFAssembler class to hold this.
+  // ELF header flags are used both by the integrated and standalone assemblers.
   // Access to the flags is necessary in cases where assembler directives affect
   // which flags to be set.
   unsigned ELFHeaderEFlags;
-  unsigned char ELFHeaderABIVersion = 0;
 
   /// Used to communicate Linker Optimization Hint information between
   /// the Streamer and the .o writer
@@ -280,10 +279,6 @@ class MCAssembler {
   unsigned getELFHeaderEFlags() const { return ELFHeaderEFlags; }
   void setELFHeaderEFlags(unsigned Flags) { ELFHeaderEFlags = Flags; }
 
-  /// ELF e_ident[EI_ABIVERSION] value
-  unsigned char getELFHeaderABIVersion() const { return ELFHeaderABIVersion; }
-  void setELFHeaderABIVersion(unsigned char V) { ELFHeaderABIVersion = V; }
-
   /// MachO deployment target version information.
   const VersionInfoType &getVersionInfo() const { return VersionInfo; }
   void setVersionMin(MCVersionMinType Type, unsigned Major, unsigned Minor,

llvm/include/llvm/MC/MCELFObjectWriter.h

@@ -52,13 +52,14 @@ struct ELFRelocationEntry {
 
 class MCELFObjectTargetWriter : public MCObjectTargetWriter {
   const uint8_t OSABI;
+  const uint8_t ABIVersion;
   const uint16_t EMachine;
   const unsigned HasRelocationAddend : 1;
   const unsigned Is64Bit : 1;
 
 protected:
   MCELFObjectTargetWriter(bool Is64Bit_, uint8_t OSABI_, uint16_t EMachine_,
-                          bool HasRelocationAddend_);
+                          bool HasRelocationAddend_, uint8_t ABIVersion_ = 0);
 
 public:
   virtual ~MCELFObjectTargetWriter() = default;
@@ -96,6 +97,7 @@ class MCELFObjectTargetWriter : public MCObjectTargetWriter {
   /// \name Accessors
   /// @{
   uint8_t getOSABI() const { return OSABI; }
+  uint8_t getABIVersion() const { return ABIVersion; }
   uint16_t getEMachine() const { return EMachine; }
   bool hasRelocationAddend() const { return HasRelocationAddend; }
   bool is64Bit() const { return Is64Bit; }

llvm/include/llvm/MC/MCObjectWriter.h

@@ -92,6 +92,9 @@ class MCObjectWriter {
   /// ELF only. Mark that we have seen GNU ABI usage (e.g. SHF_GNU_RETAIN).
   virtual void markGnuAbi() {}
 
+  /// ELF only, override the default ABIVersion in the ELF header.
+  virtual void setOverrideABIVersion(uint8_t ABIVersion) {}
+
   /// Tell the object writer to emit an address-significance table during
   /// writeObject(). If this function is not called, all symbols are treated as
   /// address-significant.

llvm/lib/MC/ELFObjectWriter.cpp

@@ -226,6 +226,8 @@ class ELFObjectWriter : public MCObjectWriter {
 
   bool SeenGnuAbi = false;
 
+  std::optional<uint8_t> OverrideABIVersion;
+
   bool hasRelocationAddend() const;
 
   bool shouldRelocateWithSymbol(const MCAssembler &Asm, const MCValue &Val,
@@ -238,6 +240,7 @@ class ELFObjectWriter : public MCObjectWriter {
 
   void reset() override {
     SeenGnuAbi = false;
+    OverrideABIVersion.reset();
     Relocations.clear();
     Renames.clear();
     MCObjectWriter::reset();
@@ -264,6 +267,10 @@ class ELFObjectWriter : public MCObjectWriter {
   void markGnuAbi() override { SeenGnuAbi = true; }
   bool seenGnuAbi() const { return SeenGnuAbi; }
 
+  bool seenOverrideABIVersion() const { return OverrideABIVersion.has_value(); }
+  uint8_t getOverrideABIVersion() const { return OverrideABIVersion.value(); }
+  void setOverrideABIVersion(uint8_t V) override { OverrideABIVersion = V; }
+
   friend struct ELFWriter;
 };
 
@@ -417,7 +424,9 @@ void ELFWriter::writeHeader(const MCAssembler &Asm) {
                    ? int(ELF::ELFOSABI_GNU)
                    : OSABI);
   // e_ident[EI_ABIVERSION]
-  W.OS << char(Asm.getELFHeaderABIVersion());
+  W.OS << char(OWriter.seenOverrideABIVersion()
+                   ? OWriter.getOverrideABIVersion()
+                   : OWriter.TargetObjectWriter->getABIVersion());
 
   W.OS.write_zeros(ELF::EI_NIDENT - ELF::EI_PAD);
 

llvm/lib/MC/MCELFObjectTargetWriter.cpp

@@ -12,8 +12,9 @@ using namespace llvm;
 
 MCELFObjectTargetWriter::MCELFObjectTargetWriter(bool Is64Bit_, uint8_t OSABI_,
                                                  uint16_t EMachine_,
-                                                 bool HasRelocationAddend_)
-    : OSABI(OSABI_), EMachine(EMachine_),
+                                                 bool HasRelocationAddend_,
+                                                 uint8_t ABIVersion_)
+    : OSABI(OSABI_), ABIVersion(ABIVersion_), EMachine(EMachine_),
       HasRelocationAddend(HasRelocationAddend_), Is64Bit(Is64Bit_) {}
 
 bool MCELFObjectTargetWriter::needsRelocateWithSymbol(const MCValue &,

llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUTargetStreamer.cpp

@@ -20,6 +20,7 @@
 #include "llvm/MC/MCAssembler.h"
 #include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCELFStreamer.h"
+#include "llvm/MC/MCObjectWriter.h"
 #include "llvm/MC/MCSectionELF.h"
 #include "llvm/MC/MCSubtargetInfo.h"
 #include "llvm/Support/AMDGPUMetadata.h"
@@ -485,7 +486,7 @@ MCELFStreamer &AMDGPUTargetELFStreamer::getStreamer() {
 void AMDGPUTargetELFStreamer::finish() {
   MCAssembler &MCA = getStreamer().getAssembler();
   MCA.setELFHeaderEFlags(getEFlags());
-  MCA.setELFHeaderABIVersion(
+  MCA.getWriter().setOverrideABIVersion(
       getELFABIVersion(STI.getTargetTriple(), CodeObjectVersion));
 
   std::string Blob;

llvm/test/MC/AMDGPU/elf-header-cov.s

@@ -4,8 +4,19 @@
 // RUN: sed 's/COV/5/g' %s | llvm-mc -triple amdgcn-amd-amdhsa -mcpu=gfx802 -filetype=obj | \
 // RUN:   llvm-readobj --file-headers - | FileCheck %s --check-prefixes=HS5
 
+// RUN: sed 's/COV/4/g' %s | not llvm-mc -triple amdgcn-amd-amdpal -mcpu=gfx802 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=ERR
+
+// RUN: sed 's/COV/4/g' %s | not llvm-mc -triple amdgcn-amd-mesa3d -mcpu=gfx802 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=ERR
+
+// RUN: sed 's/COV/4/g' %s | not llvm-mc -triple amdgcn-amd- -mcpu=gfx802 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=ERR
+
 .amdhsa_code_object_version COV
 
+// ERR: error: unknown directive
+
 // HS4: OS/ABI: AMDGPU_HSA (0x40)
 // HS4-NEXT: ABIVersion: 2