Removing Manifest from the offload binary structure.

Author: YuriPlyakhin

clang/lib/Driver/ToolChains/Clang.cpp

@@ -10097,8 +10097,13 @@ void OffloadWrapper::ConstructJob(Compilation &C, const JobAction &JA,
     // clang-offload-wrapper
     //   -o=<outputfile>.bc
     //   -host=x86_64-pc-linux-gnu -kind=sycl
+#ifndef __INTEL_PREVIEW_BREAKING_CHANGES
     //   -format=spirv <inputfile1>.spv <manifest1>(optional)
     //   -format=spirv <inputfile2>.spv <manifest2>(optional)
+#else
+    //   -format=spirv <inputfile1>.spv
+    //   -format=spirv <inputfile2>.spv
+#endif
     //  ...
     ArgStringList WrapperArgs;
 
@@ -10163,6 +10168,9 @@ void OffloadWrapper::ConstructJob(Compilation &C, const JobAction &JA,
     WrapperArgs.push_back(
         C.getArgs().MakeArgString(Twine("-kind=") + Twine(Kind)));
 
+    if (C.getArgs().hasArg(options::OPT_fpreview_breaking_changes))
+      WrapperArgs.push_back("-fpreview-breaking-changes");
+
     assert((Inputs.size() > 0) && "no inputs for clang-offload-wrapper");
     assert(((Inputs[0].getType() != types::TY_Tempfiletable) ||
             (Inputs.size() == 1)) &&
@@ -11580,6 +11588,9 @@ void LinkerWrapper::ConstructJob(Compilation &C, const JobAction &JA,
           Args.MakeArgString("--wrapper-jobs=" + Twine(NumThreads)));
   }
 
+  if (Args.hasArg(options::OPT_fpreview_breaking_changes))
+    CmdArgs.push_back("-fpreview-breaking-changes");
+
   const char *Exec =
       Args.MakeArgString(getToolChain().GetProgramPath("clang-linker-wrapper"));
 

clang/test/Driver/clang-offload-wrapper-exe-preview.cpp

@@ -0,0 +1,212 @@
+// REQUIRES: system-linux || system-windows
+
+// End-to-end clang-offload-wrapper executable test: check that -batch options
+// works, and that the tool generates data properly accessible at runtime.
+
+// --- Prepare test data
+// - create the first binary image
+// RUN: echo -e -n 'device binary image1\n' > %t.bin
+// RUN: echo -e -n '[Category1]\nint_prop1=1|10\n[Category2]\nint_prop2=1|20\n' > %t.props
+// RUN: echo -e -n 'kernel1\nkernel2\n' > %t.sym
+
+// - create the second binary image with byte array property values
+// RUN: echo -e -n 'device binary image2\n' > %t_1.bin
+// RUN: echo -e -n '[Category3]\n' > %t_1.props
+// RUN: echo -e -n 'kernel1=2|IAAAAAAAAAQA\n' >> %t_1.props
+// RUN: echo -e -n 'kernel2=2|oAAAAAAAAAw///3/wB\n' >> %t_1.props
+
+// - create the batch file input for the wrapper
+// RUN: echo '[Code|Properties|Symbols]' > %t.batch
+// RUN: echo %t.bin"|"%t.props"|"%t.sym >> %t.batch
+// RUN: echo %t_1.bin"|"%t_1.props"|" >> %t.batch
+// --- Generate "gold" output
+// RUN: cat %t.bin %t.props %t.sym > %t.all
+// --- Create the wrapper object
+// -host omitted - generate object for the host triple:
+#ifdef __INTEL_PREVIEW_BREAKING_CHANGES
+/// TODO: Remove -fpreview-breaking-changes from the command line, when removing the macro
+// RUN: clang-offload-wrapper -kind=sycl -target=TARGET -format=native -batch %t.batch -o %t.wrapped.bc -fpreview-breaking-changes
+#endif // __INTEL_PREVIEW_BREAKING_CHANGES
+// RUN: llc --filetype=obj %t.wrapped.bc -o %t.wrapped.o
+// --- Compile & link the test with the wrapper
+// RUN: %clangxx %t.wrapped.o %s -o %t.batch.exe -v
+// --- Run and check ignoring white spaces
+// RUN: %t.batch.exe > %t.batch.exe.out
+// RUN: diff -b %t.batch.exe.out %t.all
+
+#include <cassert>
+#include <cstdint>
+#include <cstring>
+#include <iostream>
+#include <string>
+
+// Data types created by the offload wrapper and inserted in the wrapper object.
+// Match those defined in SYCL runtime Plugin Interface.
+struct _pi_offload_entry_struct {
+  void *addr;
+  char *name;
+  size_t size;
+  int32_t flags;
+  int32_t reserved;
+};
+
+typedef _pi_offload_entry_struct *_pi_offload_entry;
+
+struct _pi_device_binary_property_struct {
+  char *Name;       // null-terminated property name
+  void *ValAddr;    // address of property value
+  uint32_t Type;    // pi_property_type
+  uint64_t ValSize; // size of property value in bytes
+};
+
+typedef _pi_device_binary_property_struct *pi_device_binary_property;
+
+struct _pi_device_binary_property_set_struct {
+  char *Name;                                // the name
+  pi_device_binary_property PropertiesBegin; // array start
+  pi_device_binary_property PropertiesEnd;   // array end
+};
+
+typedef _pi_device_binary_property_set_struct *pi_device_binary_property_set;
+
+struct pi_device_binary_struct {
+  uint16_t Version;
+  uint8_t Kind;   // 4 for SYCL
+  uint8_t Format; // 1 for native
+  const char *DeviceTargetSpec;
+  const char *CompileOptions;
+  const char *LinkOptions;
+  const unsigned char *BinaryStart;
+  const unsigned char *BinaryEnd;
+  _pi_offload_entry EntriesBegin;
+  _pi_offload_entry EntriesEnd;
+  pi_device_binary_property_set PropertySetsBegin;
+  pi_device_binary_property_set PropertySetsEnd;
+};
+typedef pi_device_binary_struct *pi_device_binary;
+
+struct pi_device_binaries_struct {
+  uint16_t Version;
+  uint16_t NumDeviceBinaries;
+  pi_device_binary DeviceBinaries;
+  _pi_offload_entry *HostEntriesBegin;
+  _pi_offload_entry *HostEntriesEnd;
+};
+typedef pi_device_binaries_struct *pi_device_binaries;
+
+static pi_device_binaries BinDesc = nullptr;
+
+// Wrapper object has code which calls these 2 functions below
+extern "C" void __sycl_register_lib(pi_device_binaries desc) {
+  BinDesc = desc;
+}
+
+extern "C" void __sycl_unregister_lib() {}
+
+#define ASSERT(Cond, Msg)                            \
+  if (!(Cond)) {                                     \
+    std::cerr << "*** ERROR: wrong " << Msg << "\n"; \
+    return 1;                                        \
+  }
+
+static std::string getString(const unsigned char *B, const unsigned char *E) {
+  return std::string(reinterpret_cast<const char *>(B), E - B);
+}
+
+static int getInt(void *Addr) {
+  const char *Ptr = reinterpret_cast<const char *>(Addr);
+  return Ptr[0] | (Ptr[1] << 8) | (Ptr[2] << 16) | (Ptr[3] << 24);
+}
+
+using byte = unsigned char;
+
+static void printProp(const pi_device_binary_property &Prop) {
+  std::cerr << "Property " << Prop->Name << " {\n";
+  std::cerr << "  Type: " << Prop->Type << "\n";
+  if (Prop->Type != 1)
+    std::cerr << "  Size = " << Prop->ValSize << "\n";
+
+  std::cerr << "  Value = ";
+  if (Prop->Type == 1)
+    std::cerr << getInt(&Prop->ValSize);
+  else {
+    std::cerr << " {\n   ";
+
+    byte *Ptr = (byte *)Prop->ValAddr;
+
+    for (auto I = 0; I < Prop->ValSize && I < 100; ++I) {
+      std::cerr << " 0x" << std::hex << (unsigned int)Ptr[I];
+      std::cerr << std::dec;
+    }
+    std::cerr << "\n  }";
+  }
+  std::cerr << "\n";
+  std::cerr << "}\n";
+}
+
+static int dumpBinary0() {
+  pi_device_binary Bin = &BinDesc->DeviceBinaries[0];
+  ASSERT(Bin->Kind == 4, "Bin->Kind");
+  ASSERT(Bin->Format == 1, "Bin->Format");
+
+  // dump code
+  std::cout << getString(Bin->BinaryStart, Bin->BinaryEnd);
+  // dump properties
+  for (pi_device_binary_property_set PropSet = Bin->PropertySetsBegin; PropSet != Bin->PropertySetsEnd; ++PropSet) {
+    std::cout << "[" << PropSet->Name << "]"
+              << "\n";
+
+    for (pi_device_binary_property Prop = PropSet->PropertiesBegin; Prop != PropSet->PropertiesEnd; ++Prop) {
+      ASSERT(Prop->Type == 1, "Prop->Type");
+      std::cout << Prop->Name << "=" << Prop->Type << "|" << getInt(&Prop->ValSize) << "\n";
+    }
+  }
+  // dump symbols
+  for (_pi_offload_entry Entry = Bin->EntriesBegin; Entry != Bin->EntriesEnd; ++Entry)
+    std::cout << Entry->name << "\n";
+  return 0;
+}
+
+// Clang offload wrapper does Base64 decoding on byte array property values, so
+// they can't be dumped as is and compared to the original. Instead, this
+// testcase checks that the byte array in the property value is equal to the
+// pre-decoded byte array.
+static int checkBinary1() {
+  // Decoded from "IAAAAAAAAAQA":
+  const byte Arr0[] = {8, 0, 0, 0, 0, 0, 0, 0, 0x1};
+  // Decoded from "oAAAAAAAAAw///3/wB":
+  const byte Arr1[] = {40, 0, 0, 0, 0, 0, 0, 0, 0xFF, 0xFF, 0x7F, 0xFF, 0x70};
+
+  struct {
+    const byte *Ptr;
+    const size_t Size;
+  } GoldArrays[] = {
+      {Arr0, sizeof(Arr0)},
+      {Arr1, sizeof(Arr1)}};
+  pi_device_binary Bin = &BinDesc->DeviceBinaries[1];
+  ASSERT(Bin->Kind == 4, "Bin->Kind");
+  ASSERT(Bin->Format == 1, "Bin->Format");
+
+  for (pi_device_binary_property_set PropSet = Bin->PropertySetsBegin; PropSet != Bin->PropertySetsEnd; ++PropSet) {
+    int Cnt = 0;
+
+    for (pi_device_binary_property Prop = PropSet->PropertiesBegin; Prop != PropSet->PropertiesEnd; ++Prop, ++Cnt) {
+      ASSERT(Prop->Type == 2, "Prop->Type"); // must be a byte array
+      char *Ptr = reinterpret_cast<char *>(Prop->ValAddr);
+      int Cmp = std::memcmp(Prop->ValAddr, GoldArrays[Cnt].Ptr, GoldArrays[Cnt].Size);
+      ASSERT(Cmp == 0, "byte array property");
+    }
+  }
+  return 0;
+}
+
+int main(int argc, char **argv) {
+  ASSERT(BinDesc->NumDeviceBinaries == 2, "BinDesc->NumDeviceBinaries");
+  ASSERT(BinDesc->Version == 1, "BinDesc->Version");
+
+  if (dumpBinary0() != 0)
+    return 1;
+  if (checkBinary1() != 0)
+    return 1;
+  return 0;
+}

clang/test/Driver/clang-offload-wrapper-exe.cpp

@@ -3,6 +3,11 @@
 // End-to-end clang-offload-wrapper executable test: check that -batch options
 // works, and that the tool generates data properly accessible at runtime.
 
+#ifdef __INTEL_PREVIEW_BREAKING_CHANGES
+/// TODO: Delete this test when the preview changes are enabled by default.
+/// Rename clang-offload-wrapper-exe-preview.cpp to clang-offload-wrapper-exe.cpp
+#endif // __INTEL_PREVIEW_BREAKING_CHANGES
+
 // --- Prepare test data
 // - create the first binary image
 // RUN: echo -e -n 'device binary image1\n' > %t.bin

clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp

@@ -121,6 +121,11 @@ static cl::opt<std::string> PassPipeline(
 static cl::alias PassPipeline2("p", cl::aliasopt(PassPipeline),
                                cl::desc("Alias for -passes"));
 
+static cl::opt<bool>
+    PreviewBreakingChanges("fpreview-breaking-changes",
+                           cl::desc("Enable preview breaking changes"),
+                           cl::init(false), cl::Hidden);
+
 /// Path of the current binary.
 static const char *LinkerExecutable;
 
@@ -303,7 +308,6 @@ Expected<StringRef> writeOffloadFile(const OffloadFile &File,
 
   StringRef Prefix =
       sys::path::stem(Binary.getMemoryBufferRef().getBufferIdentifier());
-  StringRef Suffix = getImageKindName(Binary.getImageKind());
 
   StringRef BinArch = (Binary.getArch() == "*") ? "any" : Binary.getArch();
   auto TempFileOrErr = createOutputFile(
@@ -1129,7 +1133,8 @@ wrapSYCLBinariesFromFile(std::vector<module_split::SplitModule> &SplitModules,
     errs() << formatv(" offload-wrapper: compile-opts: {0}, link-opts: {1}\n",
                       CompileOptions, LinkOptions);
   }
-  if (Error E = offloading::wrapSYCLBinaries(M, Images, WrappingOptions))
+  if (Error E = offloading::wrapSYCLBinaries(M, Images, WrappingOptions,
+                                             PreviewBreakingChanges))
     return E;
 
   if (Args.hasArg(OPT_print_wrapped_module))