[Binary] Promote OffloadBinary to inherit from Binary

[jhuber6]

Summary

We use the OffloadBinary to create binary images of offloading files
and their corresonding metadata. This patch changes this to inherit from
the base Binary class. This allows us to create and insepect these
more generically. This patch includes all the necessary glue to
implement this as a new binary format, along with added the magic bytes
we use to distinguish the offloading binary to the file_magic
implementation.

[jhuber6]

Fix test

[tra]

LGTM in principle, with new nits.

llvm/include/llvm/Object/OffloadBinary.h

121–123

We're losing information by passing only the start of the buffer.
I'd change the type of Buffer field to MemoryBufferRef instead.

llvm/lib/Object/OffloadBinary.cpp

18

Nit: using namespace seems a bit too heavy-handed when you only need one enum
using object_error = llvm::object::object_error; would probably do.
TBH, the use of fully qualified enum in only two instances also looked OK to me.

18

Nit: Offload binary is declared within llvm namespace. Removing namespace here and relying on using namespace llvm; works, but makes things a bit less obvious. I'd keep the namespace around. Maybe, even remove using namespace llvm above, too as it should not be needed if the code is within the actual namespace.

[jhuber6]

Thanks for the comments, I'll address them real quick.

llvm/include/llvm/Object/OffloadBinary.h

121–123

Forgot to remove this one. I just put this here for convenience's sake since I need to index the MemoryBufferRef directly as a char array. The underlying Binary type already contains the buffer ref so we're not losing it.

[jhuber6]

Addressing comments.

[tra]

llvm/lib/Object/OffloadBinary.cpp

26–27

It could be just object_error::parse_failed now. Applies to the return above, too.

llvm/unittests/Object/OffloadingTest.cpp

8

Nit: using namespace llvm::object;

[MaskRay]

llvm/lib/Object/OffloadBinary.cpp

18

using namespace llvm; is actually the preferred style. See other files in lib/Object/ and various other components.

I fixed the style in 2108f7a243a5018b4ffc09bcbc2a8bdbe3c9a4d1

[tra]

llvm/lib/Object/OffloadBinary.cpp

18

Than you. I was unaware of this:
https://llvm.org/docs/CodingStandards.html#use-namespace-qualifiers-to-implement-previously-declared-functions

clang/lib/CodeGen/BackendUtil.cpp

for (StringRef OffloadObject : CGOpts.OffloadObjects) {
     if (std::error_code EC = ObjectOrErr.getError()) {
       auto DiagID = Diags.getCustomDiagID(DiagnosticsEngine::Error,
                                           "could not open '%0' for embedding");
       Diags.Report(DiagID) << OffloadObject;
       return;
     }
 
     llvm::embedBufferInModule(*M, **ObjectOrErr, ".llvm.offloading",
-                              Align(OffloadBinary::getAlignment()));
+                              Align(object::OffloadBinary::getAlignment()));
   }
 }

llvm/include/llvm-c/Object.h

  */
 
 // Opaque type wrappers
 typedef struct LLVMOpaqueSectionIterator *LLVMSectionIteratorRef;
 typedef struct LLVMOpaqueSymbolIterator *LLVMSymbolIteratorRef;
 typedef struct LLVMOpaqueRelocationIterator *LLVMRelocationIteratorRef;
 
 typedef enum {
-  LLVMBinaryTypeArchive,                /**< Archive file. */
-  LLVMBinaryTypeMachOUniversalBinary,   /**< Mach-O Universal Binary file. */
-  LLVMBinaryTypeCOFFImportFile,         /**< COFF Import file. */
-  LLVMBinaryTypeIR,                     /**< LLVM IR. */
-  LLVMBinaryTypeWinRes,                 /**< Windows resource (.res) file. */
-  LLVMBinaryTypeCOFF,                   /**< COFF Object file. */
-  LLVMBinaryTypeELF32L,                 /**< ELF 32-bit, little endian. */
-  LLVMBinaryTypeELF32B,                 /**< ELF 32-bit, big endian. */
-  LLVMBinaryTypeELF64L,                 /**< ELF 64-bit, little endian. */
-  LLVMBinaryTypeELF64B,                 /**< ELF 64-bit, big endian. */
-  LLVMBinaryTypeMachO32L,               /**< MachO 32-bit, little endian. */
-  LLVMBinaryTypeMachO32B,               /**< MachO 32-bit, big endian. */
-  LLVMBinaryTypeMachO64L,               /**< MachO 64-bit, little endian. */
-  LLVMBinaryTypeMachO64B,               /**< MachO 64-bit, big endian. */
-  LLVMBinaryTypeWasm,                   /**< Web Assembly. */
+  LLVMBinaryTypeArchive,              /**< Archive file. */
+  LLVMBinaryTypeMachOUniversalBinary, /**< Mach-O Universal Binary file. */
+  LLVMBinaryTypeCOFFImportFile,       /**< COFF Import file. */
+  LLVMBinaryTypeIR,                   /**< LLVM IR. */
+  LLVMBinaryTypeWinRes,               /**< Windows resource (.res) file. */
+  LLVMBinaryTypeCOFF,                 /**< COFF Object file. */
+  LLVMBinaryTypeELF32L,               /**< ELF 32-bit, little endian. */
+  LLVMBinaryTypeELF32B,               /**< ELF 32-bit, big endian. */
+  LLVMBinaryTypeELF64L,               /**< ELF 64-bit, little endian. */
+  LLVMBinaryTypeELF64B,               /**< ELF 64-bit, big endian. */
+  LLVMBinaryTypeMachO32L,             /**< MachO 32-bit, little endian. */
+  LLVMBinaryTypeMachO32B,             /**< MachO 32-bit, big endian. */
+  LLVMBinaryTypeMachO64L,             /**< MachO 64-bit, little endian. */
+  LLVMBinaryTypeMachO64B,             /**< MachO 64-bit, big endian. */
+  LLVMBinaryTypeWasm,                 /**< Web Assembly. */
+  LLVMBinaryTypeOffload,              /**< Offloading fatbinary. */
+
 } LLVMBinaryType;
 
 /**
  * Create a binary file from the given memory buffer.
  *
  * The exact type of the binary file will be inferred automatically, and the
  * appropriate implementation selected.  The context may be NULL except if
  * the resulting file is an LLVM IR file.

llvm/include/llvm/BinaryFormat/Magic.h

enum Impl {
     pecoff_executable,   ///< PECOFF executable file
     windows_resource,    ///< Windows compiled resource file (.res)
     xcoff_object_32,     ///< 32-bit XCOFF object file
     xcoff_object_64,     ///< 64-bit XCOFF object file
     wasm_object,         ///< WebAssembly Object file
     pdb,                 ///< Windows PDB debug info file
     tapi_file,           ///< Text-based Dynamic Library Stub file
     cuda_fatbinary,      ///< CUDA Fatbinary object file
+    offload_binary,      ///< LLVM offload object file
     dxcontainer_object,  ///< DirectX container file
   };
 
   bool is_object() const { return V != unknown; }
 
   file_magic() = default;
   file_magic(Impl V) : V(V) {}
   operator Impl() const { return V; }

llvm/include/llvm/Object/Binary.h

enum {
 
     ID_MachO32L, // MachO 32-bit, little endian
     ID_MachO32B, // MachO 32-bit, big endian
     ID_MachO64L, // MachO 64-bit, little endian
     ID_MachO64B, // MachO 64-bit, big endian
 
     ID_Wasm,
 
+    ID_Offload, // Offloading binary file.
+
     ID_EndObjects
   };
 
   static inline unsigned int getELFType(bool isLE, bool is64Bits) {
     if (isLE)
       return is64Bits ? ID_ELF64L : ID_ELF32L;
     else
       return is64Bits ? ID_ELF64B : ID_ELF32B;
public:
   bool isCOFF() const {
     return TypeID == ID_COFF;
   }
 
   bool isXCOFF() const { return TypeID == ID_XCOFF32 || TypeID == ID_XCOFF64; }
 
   bool isWasm() const { return TypeID == ID_Wasm; }
 
+  bool isOffloadFile() const { return TypeID == ID_Offload; }
+
   bool isCOFFImportFile() const {
     return TypeID == ID_COFFImportFile;
   }
 
   bool isIR() const {
     return TypeID == ID_IR;
   }
 

llvm/include/llvm/Object/OffloadBinary.h

 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_BINARYFORMAT_OFFLOADING_H
 #define LLVM_BINARYFORMAT_OFFLOADING_H
 
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Object/Binary.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include <memory>
 
 namespace llvm {
 
+namespace object {
+
 /// The producer of the associated offloading image.
 enum OffloadKind : uint16_t {
   OFK_None = 0,
   OFK_OpenMP,
   OFK_Cuda,
   OFK_HIP,
   OFK_LAST,
 };

 /// embed the offloading image into the host executable so it can be extracted
 /// and used by the linker.
 ///
 /// Many of these could be stored in the same section by the time the linker
 /// sees it so we mark this information with a header. The version is used to
 /// detect ABI stability and the size is used to find other offloading entries
 /// that may exist in the same section. All offsets are given as absolute byte
 /// offsets from the beginning of the file.
-class OffloadBinary {
+class OffloadBinary : public Binary {
 public:
   /// The offloading metadata that will be serialized to a memory buffer.
   struct OffloadingImage {
     ImageKind TheImageKind;
     OffloadKind TheOffloadKind;
     uint32_t Flags;
     StringMap<StringRef> StringData;
     MemoryBufferRef Image;
public:
   StringRef getTriple() const { return getString("triple"); }
   StringRef getArch() const { return getString("arch"); }
   StringRef getImage() const {
     return StringRef(&Buffer[TheEntry->ImageOffset], TheEntry->ImageSize);
   }
 
   StringRef getString(StringRef Key) const { return StringData.lookup(Key); }
 
+  static bool classof(const Binary *V) { return V->isOffloadFile(); }
+
 private:
   struct Header {
     uint8_t Magic[4] = {0x10, 0xFF, 0x10, 0xAD}; // 0x10FF10AD magic bytes.
     uint32_t Version = 1;                        // Version identifier.
     uint64_t Size;        // Size in bytes of this entire binary.
     uint64_t EntryOffset; // Offset of the metadata entry in bytes.
     uint64_t EntrySize;   // Size of the metadata entry in bytes.
   };
 
   struct Entry {
     ImageKind TheImageKind;     // The kind of the image stored.
     OffloadKind TheOffloadKind; // The producer of this image.
     uint32_t Flags;             // Additional flags associated with the image.
     uint64_t StringOffset;      // Offset in bytes to the string map.
     uint64_t NumStrings;        // Number of entries in the string map.
     uint64_t ImageOffset;       // Offset in bytes of the actual binary image.
     uint64_t ImageSize;         // Size in bytes of the binary image.
   };
 
   struct StringEntry {
     uint64_t KeyOffset;
     uint64_t ValueOffset;
   };
 
-  OffloadBinary(const char *Buffer, const Header *TheHeader,
+  OffloadBinary(MemoryBufferRef Source, const Header *TheHeader,
                 const Entry *TheEntry)
-      : Buffer(Buffer), TheHeader(TheHeader), TheEntry(TheEntry) {
-
+      : Binary(Binary::ID_Offload, Source), Buffer(Source.getBufferStart()),
+        TheHeader(TheHeader), TheEntry(TheEntry) {
+    const char *Buffer = Data.getBufferStart();
     const StringEntry *StringMapBegin =
         reinterpret_cast<const StringEntry *>(&Buffer[TheEntry->StringOffset]);
     for (uint64_t I = 0, E = TheEntry->NumStrings; I != E; ++I) {
       StringRef Key = &Buffer[StringMapBegin[I].KeyOffset];
       StringData[Key] = &Buffer[StringMapBegin[I].ValueOffset];
     }
   }
 
   OffloadBinary(const OffloadBinary &Other) = delete;
 
   /// Map from keys to offsets in the binary.
   StringMap<StringRef> StringData;
-  /// Pointer to the beginning of the memory buffer for convenience.
+  /// Raw pointer to the MemoryBufferRef for convenience.
   const char *Buffer;
   /// Location of the header within the binary.
   const Header *TheHeader;
   /// Location of the metadata entries within the binary.
   const Entry *TheEntry;
 };
 
 /// Convert a string \p Name to an image kind.
 ImageKind getImageKind(StringRef Name);
 
 /// Convert an image kind to its string representation.
 StringRef getImageKindName(ImageKind Name);
 
 /// Convert a string \p Name to an offload kind.
 OffloadKind getOffloadKind(StringRef Name);
 
 /// Convert an offload kind to its string representation.
 StringRef getOffloadKindName(OffloadKind Name);
 
+} // namespace object
+
 } // namespace llvm
 #endif

llvm/lib/BinaryFormat/Magic.cpp

if (startswith(Magic, "\x01\xF7"))
       return file_magic::xcoff_object_64;
     break;
 
   case 0x03:
     if (startswith(Magic, "\x03\xF0\x00"))
       return file_magic::goff_object;
     break;
 
+  case 0x10:
+    if (startswith(Magic, "\x10\xFF\x10\xAD"))
+      return file_magic::offload_binary;
+    break;
+
   case 0xDE: // 0x0B17C0DE = BC wraper
     if (startswith(Magic, "\xDE\xC0\x17\x0B"))
       return file_magic::bitcode;
     break;
   case 'B':
     if (startswith(Magic, "BC\xC0\xDE"))
       return file_magic::bitcode;
     break;

llvm/lib/Object/Binary.cpp

 #include "llvm/Object/Binary.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/BinaryFormat/Magic.h"
 #include "llvm/Object/Archive.h"
 #include "llvm/Object/Error.h"
 #include "llvm/Object/MachOUniversal.h"
 #include "llvm/Object/Minidump.h"
 #include "llvm/Object/ObjectFile.h"
+#include "llvm/Object/OffloadBinary.h"
 #include "llvm/Object/TapiUniversal.h"
 #include "llvm/Object/WindowsResource.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include <memory>
 #include <system_error>
case file_magic::pdb:
     // PDB does not support the Binary interface.
     return errorCodeToError(object_error::invalid_file_type);
   case file_magic::unknown:
   case file_magic::cuda_fatbinary:
   case file_magic::coff_cl_gl_object:
   case file_magic::dxcontainer_object:
     // Unrecognized object file format.
     return errorCodeToError(object_error::invalid_file_type);
+  case file_magic::offload_binary:
+    return OffloadBinary::create(Buffer);
   case file_magic::minidump:
     return MinidumpFile::create(Buffer);
   case file_magic::tapi_file:
     return TapiUniversal::create(Buffer);
   }
   llvm_unreachable("Unexpected Binary File Type");
 }
 

llvm/lib/Object/Object.cpp

static LLVMBinaryType mapBinaryTypeToLLVMBinaryType(unsigned Kind) {
       case ID_MachO32L:
         return LLVMBinaryTypeMachO32L;
       case ID_MachO32B:
         return LLVMBinaryTypeMachO32B;
       case ID_MachO64L:
         return LLVMBinaryTypeMachO64L;
       case ID_MachO64B:
         return LLVMBinaryTypeMachO64B;
+      case ID_Offload:
+        return LLVMBinaryTypeOffload;
       case ID_Wasm:
         return LLVMBinaryTypeWasm;
       case ID_StartObjects:
       case ID_EndObjects:
         llvm_unreachable("Marker types are not valid binary kinds!");
       default:
         llvm_unreachable("Unknown binary kind!");
       }

llvm/lib/Object/ObjectFile.cpp

ObjectFile::createObjectFile(MemoryBufferRef Object, file_magic Type,
   case file_magic::coff_cl_gl_object:
   case file_magic::archive:
   case file_magic::macho_universal_binary:
   case file_magic::windows_resource:
   case file_magic::pdb:
   case file_magic::minidump:
   case file_magic::goff_object:
   case file_magic::cuda_fatbinary:
+  case file_magic::offload_binary:
   case file_magic::dxcontainer_object:
     return errorCodeToError(object_error::invalid_file_type);
   case file_magic::tapi_file:
     return errorCodeToError(object_error::invalid_file_type);
   case file_magic::elf:
   case file_magic::elf_relocatable:
   case file_magic::elf_executable:
   case file_magic::elf_shared_object:

llvm/lib/Object/OffloadBinary.cpp

 //===- Offloading.cpp - Utilities for handling offloading code  -*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Object/OffloadBinary.h"
 
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/BinaryFormat/Magic.h"
 #include "llvm/MC/StringTableBuilder.h"
 #include "llvm/Object/Error.h"
 #include "llvm/Support/FileOutputBuffer.h"
 
 using namespace llvm;
-
-namespace llvm {
+using namespace object;
 
 Expected<std::unique_ptr<OffloadBinary>>
 OffloadBinary::create(MemoryBufferRef Buf) {
   if (Buf.getBufferSize() < sizeof(Header) + sizeof(Entry))
-    return errorCodeToError(llvm::object::object_error::parse_failed);
+    return errorCodeToError(object_error::parse_failed);
 
   // Check for 0x10FF1OAD magic bytes.
-  if (!Buf.getBuffer().startswith("\x10\xFF\x10\xAD"))
-    return errorCodeToError(llvm::object::object_error::parse_failed);
+  if (identify_magic(Buf.getBuffer()) != file_magic::offload_binary)
+    return errorCodeToError(object_error::parse_failed);
 
   const char *Start = Buf.getBufferStart();
   const Header *TheHeader = reinterpret_cast<const Header *>(Start);
   const Entry *TheEntry =
       reinterpret_cast<const Entry *>(&Start[TheHeader->EntryOffset]);
 
   return std::unique_ptr<OffloadBinary>(
-      new OffloadBinary(Buf.getBufferStart(), TheHeader, TheEntry));
+      new OffloadBinary(Buf, TheHeader, TheEntry));
 }
 
 std::unique_ptr<MemoryBuffer>
 OffloadBinary::write(const OffloadingImage &OffloadingData) {
   // Create a null-terminated string table with all the used strings.
   StringTableBuilder StrTab(StringTableBuilder::ELF);
   for (auto &KeyAndValue : OffloadingData.StringData) {
     StrTab.add(KeyAndValue.getKey());
OffloadBinary::write(const OffloadingImage &OffloadingData) {
   // Add final padding to required alignment.
   assert(TheHeader.Size >= OS.tell() && "Too much data written?");
   OS.write_zeros(TheHeader.Size - OS.tell());
   assert(TheHeader.Size == OS.tell() && "Size mismatch");
 
   return MemoryBuffer::getMemBufferCopy(OS.str());
 }
 
-OffloadKind getOffloadKind(StringRef Name) {
+OffloadKind llvm::object::getOffloadKind(StringRef Name) {
   return llvm::StringSwitch<OffloadKind>(Name)
       .Case("openmp", OFK_OpenMP)
       .Case("cuda", OFK_Cuda)
       .Case("hip", OFK_HIP)
       .Default(OFK_None);
 }
 
-StringRef getOffloadKindName(OffloadKind Kind) {
+StringRef llvm::object::getOffloadKindName(OffloadKind Kind) {
   switch (Kind) {
   case OFK_OpenMP:
     return "openmp";
   case OFK_Cuda:
     return "cuda";
   case OFK_HIP:
     return "hip";
   default:
     return "none";
   }
 }
 
-ImageKind getImageKind(StringRef Name) {
+ImageKind llvm::object::getImageKind(StringRef Name) {
   return llvm::StringSwitch<ImageKind>(Name)
       .Case("o", IMG_Object)
       .Case("bc", IMG_Bitcode)
       .Case("cubin", IMG_Cubin)
       .Case("fatbin", IMG_Fatbinary)
       .Case("s", IMG_PTX)
       .Default(IMG_None);
 }
 
-StringRef getImageKindName(ImageKind Kind) {
+StringRef llvm::object::getImageKindName(ImageKind Kind) {
   switch (Kind) {
   case IMG_Object:
     return "o";
   case IMG_Bitcode:
     return "bc";
   case IMG_Cubin:
     return "cubin";
   case IMG_Fatbinary:
     return "fatbin";
   case IMG_PTX:
     return "s";
   default:
     return "";
   }
 }
-
-} // namespace llvm

llvm/unittests/Object/OffloadingTest.cpp

 #include "llvm/Object/OffloadBinary.h"
 
 #include "llvm/Testing/Support/Error.h"
 #include "gtest/gtest.h"
 #include <random>
 
+using namespace llvm;
+using namespace object;
+
 TEST(OffloadingTest, checkOffloadingBinary) {
   // Create random data to fill the image.
   std::mt19937 Rng(std::random_device{}());
   std::uniform_int_distribution<uint64_t> SizeDist(0, 256);
   std::uniform_int_distribution<uint16_t> KindDist(0);
   std::uniform_int_distribution<uint16_t> BinaryDist(
       std::numeric_limits<uint8_t>::min(), std::numeric_limits<uint8_t>::max());
   std::uniform_int_distribution<int16_t> StringDist('!', '~');
   std::vector<uint8_t> Image(SizeDist(Rng));
   std::generate(Image.begin(), Image.end(), [&]() { return BinaryDist(Rng); });
   std::vector<std::pair<std::string, std::string>> Strings(SizeDist(Rng));
   for (auto &KeyAndValue : Strings) {
     std::string Key(SizeDist(Rng), '\0');
     std::string Value(SizeDist(Rng), '\0');
 
     std::generate(Key.begin(), Key.end(), [&]() { return StringDist(Rng); });
     std::generate(Value.begin(), Value.end(),
                   [&]() { return StringDist(Rng); });
 
     KeyAndValue = std::make_pair(Key, Value);
   }
 
   // Create the image.
-  llvm::StringMap<llvm::StringRef> StringData;
+  StringMap<StringRef> StringData;
   for (auto &KeyAndValue : Strings)
     StringData[KeyAndValue.first] = KeyAndValue.second;
-  std::unique_ptr<llvm::MemoryBuffer> ImageData =
-      llvm::MemoryBuffer::getMemBuffer(
-          {reinterpret_cast<char *>(Image.data()), Image.size()}, "", false);
+  std::unique_ptr<MemoryBuffer> ImageData = MemoryBuffer::getMemBuffer(
 
-  llvm::OffloadBinary::OffloadingImage Data;
-  Data.TheImageKind = static_cast<llvm::ImageKind>(KindDist(Rng));
-  Data.TheOffloadKind = static_cast<llvm::OffloadKind>(KindDist(Rng));
+  OffloadBinary::OffloadingImage Data;
+  Data.TheImageKind = static_cast<ImageKind>(KindDist(Rng));
+  Data.TheOffloadKind = static_cast<OffloadKind>(KindDist(Rng));
   Data.Flags = KindDist(Rng);
   Data.StringData = StringData;
   Data.Image = *ImageData;
 
-  auto BinaryBuffer = llvm::OffloadBinary::write(Data);
+  auto BinaryBuffer = OffloadBinary::write(Data);
 
-  auto BinaryOrErr = llvm::OffloadBinary::create(*BinaryBuffer);
+  auto BinaryOrErr = OffloadBinary::create(*BinaryBuffer);
   if (!BinaryOrErr)
     FAIL();
 
   // Make sure we get the same data out.
   auto &Binary = **BinaryOrErr;
   ASSERT_EQ(Data.TheImageKind, Binary.getImageKind());
   ASSERT_EQ(Data.TheOffloadKind, Binary.getOffloadKind());
   ASSERT_EQ(Data.Flags, Binary.getFlags());
 
   for (auto &KeyAndValue : Strings)
     ASSERT_TRUE(StringData[KeyAndValue.first] ==
                 Binary.getString(KeyAndValue.first));
 
   EXPECT_TRUE(Data.Image.getBuffer() == Binary.getImage());
 
   // Ensure the size and alignment of the data is correct.
-  EXPECT_TRUE(Binary.getSize() % llvm::OffloadBinary::getAlignment() == 0);
+  EXPECT_TRUE(Binary.getSize() % OffloadBinary::getAlignment() == 0);
   EXPECT_TRUE(Binary.getSize() == BinaryBuffer->getBuffer().size());
 }