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

[ELF] Attempt to set the OS when using 'makeTriple()' #76992

Merged
merged 2 commits into from
Jan 5, 2024
Merged

Conversation

jhuber6
Copy link
Contributor

@jhuber6 jhuber6 commented Jan 4, 2024

Summary:
This patch fixes up the makeTriple() interface to emit append the
operating system information when it is readily avaialble from the ELF.
The main motivation for this is so the GPU architectures can be easily
identified correctly when given and ELF. E.g. we want
amdgpu-amd-amdhsa as the output and not amdgpu--.

This required adding support for the CUDA OS/ABI, which is easily found
to be 0x33 when using readelf.

@llvmbot
Copy link
Collaborator

llvmbot commented Jan 4, 2024

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

Author: Joseph Huber (jhuber6)

Changes

Summary:
This patch fixes up the makeTriple() interface to emit append the
operating system information when it is readily avaialble from the ELF.
The main motivation for this is so the GPU architectures can be easily
identified correctly when given and ELF. E.g. we want
amdgpu-amd-amdhsa as the output and not amdgpu--.

This required adding support for the CUDA OS/ABI, which is easily found
to be 0x33 when using readelf.


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

5 Files Affected:

  • (modified) llvm/include/llvm/BinaryFormat/ELF.h (+1)
  • (modified) llvm/include/llvm/Object/ELFObjectFile.h (+30)
  • (modified) llvm/include/llvm/Object/ObjectFile.h (+1)
  • (modified) llvm/lib/Object/ObjectFile.cpp (+9-2)
  • (modified) llvm/tools/llvm-readobj/ELFDumper.cpp (+5)
diff --git a/llvm/include/llvm/BinaryFormat/ELF.h b/llvm/include/llvm/BinaryFormat/ELF.h
index 0f968eac36e72f..9b8128a9ec4060 100644
--- a/llvm/include/llvm/BinaryFormat/ELF.h
+++ b/llvm/include/llvm/BinaryFormat/ELF.h
@@ -356,6 +356,7 @@ enum {
   ELFOSABI_AROS = 15,          // AROS
   ELFOSABI_FENIXOS = 16,       // FenixOS
   ELFOSABI_CLOUDABI = 17,      // Nuxi CloudABI
+  ELFOSABI_CUDA = 51,          // NVIDIA CUDA architecture.
   ELFOSABI_FIRST_ARCH = 64,    // First architecture-specific OS ABI
   ELFOSABI_AMDGPU_HSA = 64,    // AMD HSA runtime
   ELFOSABI_AMDGPU_PAL = 65,    // AMD PAL runtime
diff --git a/llvm/include/llvm/Object/ELFObjectFile.h b/llvm/include/llvm/Object/ELFObjectFile.h
index da78e11b678d99..da72b0e335b954 100644
--- a/llvm/include/llvm/Object/ELFObjectFile.h
+++ b/llvm/include/llvm/Object/ELFObjectFile.h
@@ -454,6 +454,7 @@ template <class ELFT> class ELFObjectFile : public ELFObjectFileBase {
   uint8_t getBytesInAddress() const override;
   StringRef getFileFormatName() const override;
   Triple::ArchType getArch() const override;
+  Triple::OSType getOS() const override;
   Expected<uint64_t> getStartAddress() const override;
 
   unsigned getPlatformFlags() const override { return EF.getHeader().e_flags; }
@@ -1382,6 +1383,35 @@ template <class ELFT> Triple::ArchType ELFObjectFile<ELFT>::getArch() const {
   }
 }
 
+template <class ELFT> Triple::OSType ELFObjectFile<ELFT>::getOS() const {
+  switch (EF.getHeader().e_ident[ELF::EI_OSABI]) {
+  case ELF::ELFOSABI_NETBSD:
+    return Triple::NetBSD;
+  case ELF::ELFOSABI_LINUX:
+    return Triple::Linux;
+  case ELF::ELFOSABI_HURD:
+    return Triple::Hurd;
+  case ELF::ELFOSABI_SOLARIS:
+    return Triple::Solaris;
+  case ELF::ELFOSABI_AIX:
+    return Triple::AIX;
+  case ELF::ELFOSABI_FREEBSD:
+    return Triple::FreeBSD;
+  case ELF::ELFOSABI_OPENBSD:
+    return Triple::OpenBSD;
+  case ELF::ELFOSABI_CUDA:
+    return Triple::CUDA;
+  case ELF::ELFOSABI_AMDGPU_HSA:
+    return Triple::AMDHSA;
+  case ELF::ELFOSABI_AMDGPU_PAL:
+    return Triple::AMDPAL;
+  case ELF::ELFOSABI_AMDGPU_MESA3D:
+    return Triple::Mesa3D;
+  default:
+    return Triple::UnknownOS;
+  }
+}
+
 template <class ELFT>
 Expected<uint64_t> ELFObjectFile<ELFT>::getStartAddress() const {
   return EF.getHeader().e_entry;
diff --git a/llvm/include/llvm/Object/ObjectFile.h b/llvm/include/llvm/Object/ObjectFile.h
index c254fc2ccfde5d..8c868c7643edcd 100644
--- a/llvm/include/llvm/Object/ObjectFile.h
+++ b/llvm/include/llvm/Object/ObjectFile.h
@@ -337,6 +337,7 @@ class ObjectFile : public SymbolicFile {
 
   virtual StringRef getFileFormatName() const = 0;
   virtual Triple::ArchType getArch() const = 0;
+  virtual Triple::OSType getOS() const { return Triple::UnknownOS; }
   virtual Expected<SubtargetFeatures> getFeatures() const = 0;
   virtual std::optional<StringRef> tryGetCPUName() const {
     return std::nullopt;
diff --git a/llvm/lib/Object/ObjectFile.cpp b/llvm/lib/Object/ObjectFile.cpp
index ca921836b7f65a..c05eb0a0468e2b 100644
--- a/llvm/lib/Object/ObjectFile.cpp
+++ b/llvm/lib/Object/ObjectFile.cpp
@@ -111,6 +111,10 @@ Triple ObjectFile::makeTriple() const {
   auto Arch = getArch();
   TheTriple.setArch(Triple::ArchType(Arch));
 
+  auto OS = getOS();
+  if (OS != Triple::UnknownOS)
+    TheTriple.setOS(OS);
+
   // For ARM targets, try to use the build attributes to build determine
   // the build target. Target features are also added, but later during
   // disassembly.
@@ -129,10 +133,13 @@ Triple ObjectFile::makeTriple() const {
     // XCOFF implies AIX.
     TheTriple.setOS(Triple::AIX);
     TheTriple.setObjectFormat(Triple::XCOFF);
-  }
-  else if (isGOFF()) {
+  } else if (isGOFF()) {
     TheTriple.setOS(Triple::ZOS);
     TheTriple.setObjectFormat(Triple::GOFF);
+  } else if (TheTriple.isAMDGPU()) {
+    TheTriple.setVendor(Triple::AMD);
+  } else if (TheTriple.isNVPTX()) {
+    TheTriple.setVendor(Triple::NVIDIA);
   }
 
   return TheTriple;
diff --git a/llvm/tools/llvm-readobj/ELFDumper.cpp b/llvm/tools/llvm-readobj/ELFDumper.cpp
index abf7ba6ba1c387..10797b83d3d906 100644
--- a/llvm/tools/llvm-readobj/ELFDumper.cpp
+++ b/llvm/tools/llvm-readobj/ELFDumper.cpp
@@ -1084,6 +1084,7 @@ const EnumEntry<unsigned> ElfOSABI[] = {
   {"AROS",         "AROS",                 ELF::ELFOSABI_AROS},
   {"FenixOS",      "FenixOS",              ELF::ELFOSABI_FENIXOS},
   {"CloudABI",     "CloudABI",             ELF::ELFOSABI_CLOUDABI},
+  {"CUDA",         "NVIDIA - CUDA",        ELF::ELFOSABI_CUDA},
   {"Standalone",   "Standalone App",       ELF::ELFOSABI_STANDALONE}
 };
 
@@ -1093,6 +1094,10 @@ const EnumEntry<unsigned> AMDGPUElfOSABI[] = {
   {"AMDGPU_MESA3D", "AMDGPU - MESA3D", ELF::ELFOSABI_AMDGPU_MESA3D}
 };
 
+const EnumEntry<unsigned> NVPTXElfOSABI[] = {
+  {"NVIDIA_CUDA", "NVIDIA - CUDA", ELF::ELFOSABI_CUDA},
+};
+
 const EnumEntry<unsigned> ARMElfOSABI[] = {
   {"ARM", "ARM", ELF::ELFOSABI_ARM}
 };

Copy link

github-actions bot commented Jan 4, 2024

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff 166bd4e1f18da221621953bd5943c1a8d17201fe a8dd522d9463a872288719cde3f3d22b4404d33d -- llvm/include/llvm/BinaryFormat/ELF.h llvm/include/llvm/Object/ELFObjectFile.h llvm/include/llvm/Object/ObjectFile.h llvm/lib/Object/ObjectFile.cpp llvm/tools/llvm-readobj/ELFDumper.cpp llvm/unittests/Object/ELFObjectFileTest.cpp
View the diff from clang-format here.
diff --git a/llvm/tools/llvm-readobj/ELFDumper.cpp b/llvm/tools/llvm-readobj/ELFDumper.cpp
index 10797b83d3..b38baccce0 100644
--- a/llvm/tools/llvm-readobj/ELFDumper.cpp
+++ b/llvm/tools/llvm-readobj/ELFDumper.cpp
@@ -1067,26 +1067,25 @@ const EnumEntry<unsigned> ElfObjectFileType[] = {
 };
 
 const EnumEntry<unsigned> ElfOSABI[] = {
-  {"SystemV",      "UNIX - System V",      ELF::ELFOSABI_NONE},
-  {"HPUX",         "UNIX - HP-UX",         ELF::ELFOSABI_HPUX},
-  {"NetBSD",       "UNIX - NetBSD",        ELF::ELFOSABI_NETBSD},
-  {"GNU/Linux",    "UNIX - GNU",           ELF::ELFOSABI_LINUX},
-  {"GNU/Hurd",     "GNU/Hurd",             ELF::ELFOSABI_HURD},
-  {"Solaris",      "UNIX - Solaris",       ELF::ELFOSABI_SOLARIS},
-  {"AIX",          "UNIX - AIX",           ELF::ELFOSABI_AIX},
-  {"IRIX",         "UNIX - IRIX",          ELF::ELFOSABI_IRIX},
-  {"FreeBSD",      "UNIX - FreeBSD",       ELF::ELFOSABI_FREEBSD},
-  {"TRU64",        "UNIX - TRU64",         ELF::ELFOSABI_TRU64},
-  {"Modesto",      "Novell - Modesto",     ELF::ELFOSABI_MODESTO},
-  {"OpenBSD",      "UNIX - OpenBSD",       ELF::ELFOSABI_OPENBSD},
-  {"OpenVMS",      "VMS - OpenVMS",        ELF::ELFOSABI_OPENVMS},
-  {"NSK",          "HP - Non-Stop Kernel", ELF::ELFOSABI_NSK},
-  {"AROS",         "AROS",                 ELF::ELFOSABI_AROS},
-  {"FenixOS",      "FenixOS",              ELF::ELFOSABI_FENIXOS},
-  {"CloudABI",     "CloudABI",             ELF::ELFOSABI_CLOUDABI},
-  {"CUDA",         "NVIDIA - CUDA",        ELF::ELFOSABI_CUDA},
-  {"Standalone",   "Standalone App",       ELF::ELFOSABI_STANDALONE}
-};
+    {"SystemV", "UNIX - System V", ELF::ELFOSABI_NONE},
+    {"HPUX", "UNIX - HP-UX", ELF::ELFOSABI_HPUX},
+    {"NetBSD", "UNIX - NetBSD", ELF::ELFOSABI_NETBSD},
+    {"GNU/Linux", "UNIX - GNU", ELF::ELFOSABI_LINUX},
+    {"GNU/Hurd", "GNU/Hurd", ELF::ELFOSABI_HURD},
+    {"Solaris", "UNIX - Solaris", ELF::ELFOSABI_SOLARIS},
+    {"AIX", "UNIX - AIX", ELF::ELFOSABI_AIX},
+    {"IRIX", "UNIX - IRIX", ELF::ELFOSABI_IRIX},
+    {"FreeBSD", "UNIX - FreeBSD", ELF::ELFOSABI_FREEBSD},
+    {"TRU64", "UNIX - TRU64", ELF::ELFOSABI_TRU64},
+    {"Modesto", "Novell - Modesto", ELF::ELFOSABI_MODESTO},
+    {"OpenBSD", "UNIX - OpenBSD", ELF::ELFOSABI_OPENBSD},
+    {"OpenVMS", "VMS - OpenVMS", ELF::ELFOSABI_OPENVMS},
+    {"NSK", "HP - Non-Stop Kernel", ELF::ELFOSABI_NSK},
+    {"AROS", "AROS", ELF::ELFOSABI_AROS},
+    {"FenixOS", "FenixOS", ELF::ELFOSABI_FENIXOS},
+    {"CloudABI", "CloudABI", ELF::ELFOSABI_CLOUDABI},
+    {"CUDA", "NVIDIA - CUDA", ELF::ELFOSABI_CUDA},
+    {"Standalone", "Standalone App", ELF::ELFOSABI_STANDALONE}};
 
 const EnumEntry<unsigned> AMDGPUElfOSABI[] = {
   {"AMDGPU_HSA",    "AMDGPU - HSA",    ELF::ELFOSABI_AMDGPU_HSA},
@@ -1095,7 +1094,7 @@ const EnumEntry<unsigned> AMDGPUElfOSABI[] = {
 };
 
 const EnumEntry<unsigned> NVPTXElfOSABI[] = {
-  {"NVIDIA_CUDA", "NVIDIA - CUDA", ELF::ELFOSABI_CUDA},
+    {"NVIDIA_CUDA", "NVIDIA - CUDA", ELF::ELFOSABI_CUDA},
 };
 
 const EnumEntry<unsigned> ARMElfOSABI[] = {

@jhuber6
Copy link
Contributor Author

jhuber6 commented Jan 4, 2024

I intentionally avoided reformatting the llvm-readobj code to better match the preexisting style.

Copy link
Contributor

@arsenm arsenm left a comment

Choose a reason for hiding this comment

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

Is this testable?

@jhuber6
Copy link
Contributor Author

jhuber6 commented Jan 5, 2024

Is this testable?

There's not any code that tests it right now. I think it's annoying to test because it requires a unit test and a working binary. For non-NVPTX (because of PTX) we could probably just run the backend on an empty LLVM-IR file if nothing else if we really wanted to test it. It's a lot of effort however.

There's a use of this in llvm-objdump but it doesn't print it anywhere we could check it doesn't seem.

Summary:
This patch fixes up the `makeTriple()` interface to emit append the
operating system information when it is readily avaialble from the ELF.
The main motivation for this is so the GPU architectures can be easily
identified correctly when given and ELF. E.g. we want
`amdgpu-amd-amdhsa` as the output and not `amdgpu--`.

This required adding support for the CUDA OS/ABI, which is easily found
to be `0x33` when using `readelf`.
@jhuber6 jhuber6 merged commit 3b337bb into llvm:main Jan 5, 2024
3 of 4 checks passed
@MaskRay
Copy link
Member

MaskRay commented Jan 6, 2024

LGTM

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.

None yet

5 participants