[SYCLBIN] Draft for moving to new offload binary format

clang/include/clang/Options/Options.td

@@ -7593,6 +7593,7 @@ def fsycl_dump_device_code_EQ : Joined<["-"], "fsycl-dump-device-code=">,
   Alias<save_offload_code_EQ>,
   Flags<[NoXarchOption, Deprecated]>,
   HelpText<"Dump device code into the user provided directory. (deprecated)">;
+  // we will need to stop using SYCLBIN term and use kernel_bundle state instead
 def fsyclbin_EQ : Joined<["-"], "fsyclbin=">, Values<"executable,object,input">,
   HelpText<"Output in the SYCLBIN binary format in the state specified by <arg> (input, object or executable (default))">;
 def fsyclbin : Flag<["-"], "fsyclbin">, Alias<fsyclbin_EQ>,

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

@@ -1221,6 +1221,8 @@ static Expected<StringRef> runCompile(StringRef &InputFile,
 
 /// Write an OffloadBinary containing the serialized SYCLBIN resulting from
 /// \p ModuleDescs to the ExecutableName file with the .syclbin extension.
+// Rewrite this function. SYCLBIN serialization would return OffloadBinary,
+// no need to serialize SYCLBIN and then wrap into OffloadBinary as an entry.
 static Expected<StringRef>
 packageSYCLBIN(SYCLBIN::BundleState State,
                const ArrayRef<SYCLBIN::SYCLBINModuleDesc> Modules) {
@@ -1275,6 +1277,7 @@ Error copyFileToFinalExecutable(StringRef File, const ArgList &Args) {
   return Error::success();
 }
 
+// if we use OffloadBinary instead of SYCLBIN, do we get merging for free???
 Error mergeSYCLBIN(ArrayRef<StringRef> Files, const ArgList &Args) {
   // Fast path for the general case where there's only one file. In this case we
   // do not need to parse it and can instead simply copy it.

llvm/include/llvm/Object/OffloadBinary.h

@@ -49,7 +49,7 @@ enum ImageKind : uint16_t {
   IMG_Fatbinary,
   IMG_PTX,
   IMG_SPIRV,
-  IMG_SYCLBIN,
+  IMG_SYCLBIN, // this will not be needed.
   IMG_LAST,
 };
 

llvm/include/llvm/Object/SYCLBIN.h

@@ -20,6 +20,8 @@ namespace object {
 
 // Representation of a SYCLBIN binary object. This is intended for use as an
 // image inside a OffloadBinary.
+// should we name it kernel_bundle or something like that?
+// should we inherit it from OffloadBinary? What would we actually need on top of offload binary???
 class SYCLBIN {
 public:
   SYCLBIN(MemoryBufferRef Source) : Data{Source} {}
@@ -40,6 +42,7 @@ class SYCLBIN {
     std::vector<module_split::SplitModule> SplitModules;
   };
 
+  // this class will need to be updated
   class SYCLBINDesc {
   public:
     SYCLBINDesc(BundleState State, ArrayRef<SYCLBINModuleDesc> ModuleDescs);
@@ -73,15 +76,19 @@ class SYCLBIN {
   };
 
   /// The current version of the binary used for backwards compatibility.
+  // this we would deprecate and remove later...
+  // Basically, syclbin as format would be discontinued.
   static constexpr uint32_t CurrentVersion = 1;
 
   /// Magic number used to identify SYCLBIN files.
   static constexpr uint32_t MagicNumber = 0x53594249;
 
   /// Serialize \p Desc to \p OS .
+  // this would need to be updated. We would need to support 2 formats for some time...
   static Error write(const SYCLBIN::SYCLBINDesc &Desc, raw_ostream &OS);
 
   /// Deserialize the contents of \p Source to produce a SYCLBIN object.
+  // this would need to be updated. We would need to support 2 formats for some time...
   static Expected<std::unique_ptr<SYCLBIN>> read(MemoryBufferRef Source);
 
   struct IRModule {
@@ -104,6 +111,8 @@ class SYCLBIN {
   SmallVector<AbstractModule, 4> AbstractModules;
 
 private:
+// I guess we can keep all these structures below for now for prototype
+// but for final implementation and upstreaming we should just use offload binary directly...
   MemoryBufferRef Data;
 
   struct alignas(8) FileHeaderType {

llvm/include/llvm/Support/PropertySetIO.h

@@ -230,6 +230,7 @@ class PropertySetRegistry {
       "reqd_work_group_size_uint64_t";
 
   // SYCLBIN specific property sets.
+  //how would we name it???
   static constexpr char SYCLBIN_GLOBAL_METADATA[] = "SYCLBIN/global metadata";
   static constexpr char SYCLBIN_IR_MODULE_METADATA[] =
       "SYCLBIN/ir module metadata";

llvm/lib/Object/OffloadBinary.cpp

@@ -327,7 +327,7 @@ ImageKind object::getImageKind(StringRef Name) {
       .Case("cubin", IMG_Cubin)
       .Case("fatbin", IMG_Fatbinary)
       .Case("s", IMG_PTX)
-      .Case("syclbin", IMG_SYCLBIN)
+      .Case("syclbin", IMG_SYCLBIN) // that would be removed.
       .Default(IMG_None);
 }
 
@@ -343,7 +343,7 @@ StringRef object::getImageKindName(ImageKind Kind) {
     return "fatbin";
   case IMG_PTX:
     return "s";
-  case IMG_SYCLBIN:
+  case IMG_SYCLBIN: // that would be removed.
     return "syclbin";
   default:
     return "";

llvm/lib/Object/SYCLBIN.cpp

@@ -17,6 +17,8 @@
 using namespace llvm;
 using namespace llvm::object;
 
+// code here would need to be updated. We would need to support 2 formats for some time...
+
 namespace {
 
 class SYCLBINBlockReader {

llvm/unittests/Object/SYCLBINTest.cpp

@@ -5,7 +5,7 @@
 #include "llvm/Testing/Support/Error.h"
 #include "gtest/gtest.h"
 #include <random>
-
+// new test would be needed probably...
 using namespace llvm;
 using namespace llvm::sys;
 using namespace llvm::object;

sycl/CMakeLists.txt

@@ -412,7 +412,7 @@ add_custom_target( sycl-toolchain ALL
   DEPENDS sycl-runtime-libraries
           sycl-compiler
           sycl-ls
-          syclbin-dump
+          syclbin-dump   # would we need it?
           ${XPTIFW_LIBS}
   COMMENT "Building SYCL compiler toolchain..."
 )

sycl/ReleaseNotes.md

@@ -272,9 +272,11 @@ Release notes for commit range
 
 ### Documentation
 
+This extension would need to be updated:
 - Proposed the
   [`sycl_ext_oneapi_syclbin`](https://github.com/intel/llvm/blob/b23d69e2c3fda1d69351137991897c96bf6a586d/sycl/doc/extensions/proposed/sycl_ext_oneapi_syclbin.asciidoc)
   extension. intel/llvm#16784
+
 - Updated the
   [`sycl_ext_intel_device_info`](https://github.com/intel/llvm/blob/b23d69e2c3fda1d69351137991897c96bf6a586d/sycl/doc/extensions/supported/sycl_ext_intel_device_info.md)
   extension specification to clarify that no additional environment variables