[clang] [ClangOffloadBundler] Add file size to header (PR #88827)

2024-04-19 Thread Yaxun Liu via cfe-commits

https://github.com/yxsamliu closed 
https://github.com/llvm/llvm-project/pull/88827
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [ClangOffloadBundler] Add file size to header (PR #88827)

2024-04-17 Thread Yaxun Liu via cfe-commits

yxsamliu wrote:

> Seems the documentation builder is complaining, maybe something wrong with 
> the .rst file.

It is passing now

https://github.com/llvm/llvm-project/pull/88827
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [ClangOffloadBundler] Add file size to header (PR #88827)

2024-04-16 Thread Yaxun Liu via cfe-commits

https://github.com/yxsamliu updated 
https://github.com/llvm/llvm-project/pull/88827

>From 7574258faa28a43549e507d08128ba700c42acc8 Mon Sep 17 00:00:00 2001
From: "Yaxun (Sam) Liu" 
Date: Mon, 15 Apr 2024 18:02:42 -0400
Subject: [PATCH] [ClangOffloadBundler] Add file size to header

__hipRegisterFatBinary only accepts one pointer argument. It is
expected to get the fat binary size from the header.

This patch adds a file size field to the header of the compressed
bundle.
---
 clang/docs/ClangOffloadBundler.rst|  5 +-
 clang/include/clang/Driver/OffloadBundler.h   | 17 +++---
 clang/lib/Driver/OffloadBundler.cpp   | 54 +--
 .../test/Driver/clang-offload-bundler-zstd.c  | 19 ---
 4 files changed, 64 insertions(+), 31 deletions(-)

diff --git a/clang/docs/ClangOffloadBundler.rst 
b/clang/docs/ClangOffloadBundler.rst
index 1f8c85a08f8c79..3214ba996e47da 100644
--- a/clang/docs/ClangOffloadBundler.rst
+++ b/clang/docs/ClangOffloadBundler.rst
@@ -518,7 +518,10 @@ The compressed offload bundle begins with a header 
followed by the compressed bi
 This is a unique identifier to distinguish compressed offload bundles. The 
value is the string 'CCOB' (Compressed Clang Offload Bundle).
 
 - **Version Number (16-bit unsigned int)**:
-This denotes the version of the compressed offload bundle format. The 
current version is `1`.
+This denotes the version of the compressed offload bundle format. The 
current version is `2`.
+
+- **Total File Size (32-bit unsigned int)**:
+This is the total size (in bytes) of the file, including the header. 
Available in version 2 and above.
 
 - **Compression Method (16-bit unsigned int)**:
 This field indicates the compression method used. The value corresponds to 
either `zlib` or `zstd`, represented as a 16-bit unsigned integer cast from the 
LLVM compression enumeration.
diff --git a/clang/include/clang/Driver/OffloadBundler.h 
b/clang/include/clang/Driver/OffloadBundler.h
index 65d33bfbd2825f..dcda2b40fe333f 100644
--- a/clang/include/clang/Driver/OffloadBundler.h
+++ b/clang/include/clang/Driver/OffloadBundler.h
@@ -98,6 +98,7 @@ struct OffloadTargetInfo {
 // The format is as follows:
 // - Magic Number (4 bytes) - A constant "CCOB".
 // - Version (2 bytes)
+// - Total file size (4 bytes). Available in version 2 and above.
 // - Compression Method (2 bytes) - Uses the values from
 // llvm::compression::Format.
 // - Uncompressed Size (4 bytes).
@@ -108,14 +109,18 @@ class CompressedOffloadBundle {
 private:
   static inline const size_t MagicSize = 4;
   static inline const size_t VersionFieldSize = sizeof(uint16_t);
+  static inline const size_t FileSizeFieldSize = sizeof(uint32_t);
   static inline const size_t MethodFieldSize = sizeof(uint16_t);
-  static inline const size_t SizeFieldSize = sizeof(uint32_t);
-  static inline const size_t HashFieldSize = 8;
-  static inline const size_t HeaderSize = MagicSize + VersionFieldSize +
-  MethodFieldSize + SizeFieldSize +
-  HashFieldSize;
+  static inline const size_t UncompressedSizeFieldSize = sizeof(uint32_t);
+  static inline const size_t HashFieldSize = sizeof(uint64_t);
+  static inline const size_t V1HeaderSize =
+  MagicSize + VersionFieldSize + MethodFieldSize +
+  UncompressedSizeFieldSize + HashFieldSize;
+  static inline const size_t V2HeaderSize =
+  MagicSize + VersionFieldSize + FileSizeFieldSize + MethodFieldSize +
+  UncompressedSizeFieldSize + HashFieldSize;
   static inline const llvm::StringRef MagicNumber = "CCOB";
-  static inline const uint16_t Version = 1;
+  static inline const uint16_t Version = 2;
 
 public:
   static llvm::Expected>
diff --git a/clang/lib/Driver/OffloadBundler.cpp 
b/clang/lib/Driver/OffloadBundler.cpp
index 77c89356bc76bb..b68b3c2d5df35a 100644
--- a/clang/lib/Driver/OffloadBundler.cpp
+++ b/clang/lib/Driver/OffloadBundler.cpp
@@ -1010,11 +1010,17 @@ 
CompressedOffloadBundle::compress(llvm::compression::Params P,
 
   uint16_t CompressionMethod = static_cast(P.format);
   uint32_t UncompressedSize = Input.getBuffer().size();
+  uint32_t TotalFileSize = MagicNumber.size() + sizeof(TotalFileSize) +
+   sizeof(Version) + sizeof(CompressionMethod) +
+   sizeof(UncompressedSize) + sizeof(TruncatedHash) +
+   CompressedBuffer.size();
 
   SmallVector FinalBuffer;
   llvm::raw_svector_ostream OS(FinalBuffer);
   OS << MagicNumber;
   OS.write(reinterpret_cast(), sizeof(Version));
+  OS.write(reinterpret_cast(),
+   sizeof(TotalFileSize));
   OS.write(reinterpret_cast(),
sizeof(CompressionMethod));
   OS.write(reinterpret_cast(),
@@ -1034,6 +1040,8 @@ 
CompressedOffloadBundle::compress(llvm::compression::Params P,
 (UncompressedSize / (1024.0 * 1024.0)) / CompressionTimeSeconds;
 
 llvm::errs() << "Compressed bundle format version: " << 

[clang] [ClangOffloadBundler] Add file size to header (PR #88827)

2024-04-16 Thread Joseph Huber via cfe-commits

jhuber6 wrote:

Seems the documentation builder is complaining, maybe something wrong with the 
.rst file.

https://github.com/llvm/llvm-project/pull/88827
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [ClangOffloadBundler] Add file size to header (PR #88827)

2024-04-16 Thread Joseph Huber via cfe-commits

jhuber6 wrote:

> > Isn't this ABI breaking since we're changing the size of the struct? 
> > Shouldn't that necessitate a new version?
> > Also unrelated, I wonder if there's a future where we can use the 
> > ClangOffloadPackager format in the HIP runtime.
> 
> I think you are right. Although this feature has not been used extensively, 
> it is better to bump up the version to keep backward compatibility.
> 
> Currently we are moving the extraction of code object from fat binary from 
> runtime to comgr. We can treat the offload packager format as another flavor 
> of fat binary. We only need to pass a pointer to sufficient information to 
> __hipRegisterFatBinary, and let runtime pass that pointer to comgr, then 
> comgr do the extraction. comgr just needs to identify it is offload packager 
> format and handle it properly.

Yeah, it's got magic bytes so it should be fairly trivial so long as someone 
includes the code to extract it (Which is pretty simple as well).

https://github.com/llvm/llvm-project/pull/88827
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [ClangOffloadBundler] Add file size to header (PR #88827)

2024-04-16 Thread Yaxun Liu via cfe-commits

https://github.com/yxsamliu updated 
https://github.com/llvm/llvm-project/pull/88827

>From fec9509f0c9162331fd2a7757e74b8c8408990c0 Mon Sep 17 00:00:00 2001
From: "Yaxun (Sam) Liu" 
Date: Mon, 15 Apr 2024 18:02:42 -0400
Subject: [PATCH] [ClangOffloadBundler] Add file size to header

__hipRegisterFatBinary only accepts one pointer argument. It is
expected to get the fat binary size from the header.

This patch adds a file size field to the header of the compressed
bundle.
---
 clang/docs/ClangOffloadBundler.rst|  5 +-
 clang/include/clang/Driver/OffloadBundler.h   | 17 +++---
 clang/lib/Driver/OffloadBundler.cpp   | 54 +--
 .../test/Driver/clang-offload-bundler-zstd.c  | 19 ---
 4 files changed, 64 insertions(+), 31 deletions(-)

diff --git a/clang/docs/ClangOffloadBundler.rst 
b/clang/docs/ClangOffloadBundler.rst
index 1f8c85a08f8c79..3214ba996e47da 100644
--- a/clang/docs/ClangOffloadBundler.rst
+++ b/clang/docs/ClangOffloadBundler.rst
@@ -518,7 +518,10 @@ The compressed offload bundle begins with a header 
followed by the compressed bi
 This is a unique identifier to distinguish compressed offload bundles. The 
value is the string 'CCOB' (Compressed Clang Offload Bundle).
 
 - **Version Number (16-bit unsigned int)**:
-This denotes the version of the compressed offload bundle format. The 
current version is `1`.
+This denotes the version of the compressed offload bundle format. The 
current version is `2`.
+
+- **Total File Size (32-bit unsigned int)**:
+This is the total size (in bytes) of the file, including the header. 
Available in version 2 and above.
 
 - **Compression Method (16-bit unsigned int)**:
 This field indicates the compression method used. The value corresponds to 
either `zlib` or `zstd`, represented as a 16-bit unsigned integer cast from the 
LLVM compression enumeration.
diff --git a/clang/include/clang/Driver/OffloadBundler.h 
b/clang/include/clang/Driver/OffloadBundler.h
index 65d33bfbd2825f..dcda2b40fe333f 100644
--- a/clang/include/clang/Driver/OffloadBundler.h
+++ b/clang/include/clang/Driver/OffloadBundler.h
@@ -98,6 +98,7 @@ struct OffloadTargetInfo {
 // The format is as follows:
 // - Magic Number (4 bytes) - A constant "CCOB".
 // - Version (2 bytes)
+// - Total file size (4 bytes). Available in version 2 and above.
 // - Compression Method (2 bytes) - Uses the values from
 // llvm::compression::Format.
 // - Uncompressed Size (4 bytes).
@@ -108,14 +109,18 @@ class CompressedOffloadBundle {
 private:
   static inline const size_t MagicSize = 4;
   static inline const size_t VersionFieldSize = sizeof(uint16_t);
+  static inline const size_t FileSizeFieldSize = sizeof(uint32_t);
   static inline const size_t MethodFieldSize = sizeof(uint16_t);
-  static inline const size_t SizeFieldSize = sizeof(uint32_t);
-  static inline const size_t HashFieldSize = 8;
-  static inline const size_t HeaderSize = MagicSize + VersionFieldSize +
-  MethodFieldSize + SizeFieldSize +
-  HashFieldSize;
+  static inline const size_t UncompressedSizeFieldSize = sizeof(uint32_t);
+  static inline const size_t HashFieldSize = sizeof(uint64_t);
+  static inline const size_t V1HeaderSize =
+  MagicSize + VersionFieldSize + MethodFieldSize +
+  UncompressedSizeFieldSize + HashFieldSize;
+  static inline const size_t V2HeaderSize =
+  MagicSize + VersionFieldSize + FileSizeFieldSize + MethodFieldSize +
+  UncompressedSizeFieldSize + HashFieldSize;
   static inline const llvm::StringRef MagicNumber = "CCOB";
-  static inline const uint16_t Version = 1;
+  static inline const uint16_t Version = 2;
 
 public:
   static llvm::Expected>
diff --git a/clang/lib/Driver/OffloadBundler.cpp 
b/clang/lib/Driver/OffloadBundler.cpp
index 77c89356bc76bb..b68b3c2d5df35a 100644
--- a/clang/lib/Driver/OffloadBundler.cpp
+++ b/clang/lib/Driver/OffloadBundler.cpp
@@ -1010,11 +1010,17 @@ 
CompressedOffloadBundle::compress(llvm::compression::Params P,
 
   uint16_t CompressionMethod = static_cast(P.format);
   uint32_t UncompressedSize = Input.getBuffer().size();
+  uint32_t TotalFileSize = MagicNumber.size() + sizeof(TotalFileSize) +
+   sizeof(Version) + sizeof(CompressionMethod) +
+   sizeof(UncompressedSize) + sizeof(TruncatedHash) +
+   CompressedBuffer.size();
 
   SmallVector FinalBuffer;
   llvm::raw_svector_ostream OS(FinalBuffer);
   OS << MagicNumber;
   OS.write(reinterpret_cast(), sizeof(Version));
+  OS.write(reinterpret_cast(),
+   sizeof(TotalFileSize));
   OS.write(reinterpret_cast(),
sizeof(CompressionMethod));
   OS.write(reinterpret_cast(),
@@ -1034,6 +1040,8 @@ 
CompressedOffloadBundle::compress(llvm::compression::Params P,
 (UncompressedSize / (1024.0 * 1024.0)) / CompressionTimeSeconds;
 
 llvm::errs() << "Compressed bundle format version: " << 

[clang] [ClangOffloadBundler] Add file size to header (PR #88827)

2024-04-15 Thread Yaxun Liu via cfe-commits

https://github.com/yxsamliu updated 
https://github.com/llvm/llvm-project/pull/88827

>From d18a8c971d0220bafad643bb883b90c03f483913 Mon Sep 17 00:00:00 2001
From: "Yaxun (Sam) Liu" 
Date: Mon, 15 Apr 2024 18:02:42 -0400
Subject: [PATCH] [ClangOffloadBundler] Add file size to header

__hipRegisterFatBinary only accepts one pointer argument. It is
expected to get the fat binary size from the header.

This patch adds a file size field to the header of the compressed
bundle.
---
 clang/docs/ClangOffloadBundler.rst|  5 +-
 clang/include/clang/Driver/OffloadBundler.h   | 17 +++---
 clang/lib/Driver/OffloadBundler.cpp   | 54 +--
 .../test/Driver/clang-offload-bundler-zstd.c  | 19 ---
 4 files changed, 64 insertions(+), 31 deletions(-)

diff --git a/clang/docs/ClangOffloadBundler.rst 
b/clang/docs/ClangOffloadBundler.rst
index 1f8c85a08f8c79..3214ba996e47da 100644
--- a/clang/docs/ClangOffloadBundler.rst
+++ b/clang/docs/ClangOffloadBundler.rst
@@ -518,7 +518,10 @@ The compressed offload bundle begins with a header 
followed by the compressed bi
 This is a unique identifier to distinguish compressed offload bundles. The 
value is the string 'CCOB' (Compressed Clang Offload Bundle).
 
 - **Version Number (16-bit unsigned int)**:
-This denotes the version of the compressed offload bundle format. The 
current version is `1`.
+This denotes the version of the compressed offload bundle format. The 
current version is `2`.
+
+- **Total File Size (32-bit unsigned int)**:
+This is the total size (in bytes) of the file, including the header. 
Available in version 2 and above.
 
 - **Compression Method (16-bit unsigned int)**:
 This field indicates the compression method used. The value corresponds to 
either `zlib` or `zstd`, represented as a 16-bit unsigned integer cast from the 
LLVM compression enumeration.
diff --git a/clang/include/clang/Driver/OffloadBundler.h 
b/clang/include/clang/Driver/OffloadBundler.h
index 65d33bfbd2825f..dcda2b40fe333f 100644
--- a/clang/include/clang/Driver/OffloadBundler.h
+++ b/clang/include/clang/Driver/OffloadBundler.h
@@ -98,6 +98,7 @@ struct OffloadTargetInfo {
 // The format is as follows:
 // - Magic Number (4 bytes) - A constant "CCOB".
 // - Version (2 bytes)
+// - Total file size (4 bytes). Available in version 2 and above.
 // - Compression Method (2 bytes) - Uses the values from
 // llvm::compression::Format.
 // - Uncompressed Size (4 bytes).
@@ -108,14 +109,18 @@ class CompressedOffloadBundle {
 private:
   static inline const size_t MagicSize = 4;
   static inline const size_t VersionFieldSize = sizeof(uint16_t);
+  static inline const size_t FileSizeFieldSize = sizeof(uint32_t);
   static inline const size_t MethodFieldSize = sizeof(uint16_t);
-  static inline const size_t SizeFieldSize = sizeof(uint32_t);
-  static inline const size_t HashFieldSize = 8;
-  static inline const size_t HeaderSize = MagicSize + VersionFieldSize +
-  MethodFieldSize + SizeFieldSize +
-  HashFieldSize;
+  static inline const size_t UncompressedSizeFieldSize = sizeof(uint32_t);
+  static inline const size_t HashFieldSize = sizeof(uint64_t);
+  static inline const size_t V1HeaderSize =
+  MagicSize + VersionFieldSize + MethodFieldSize +
+  UncompressedSizeFieldSize + HashFieldSize;
+  static inline const size_t V2HeaderSize =
+  MagicSize + VersionFieldSize + FileSizeFieldSize + MethodFieldSize +
+  UncompressedSizeFieldSize + HashFieldSize;
   static inline const llvm::StringRef MagicNumber = "CCOB";
-  static inline const uint16_t Version = 1;
+  static inline const uint16_t Version = 2;
 
 public:
   static llvm::Expected>
diff --git a/clang/lib/Driver/OffloadBundler.cpp 
b/clang/lib/Driver/OffloadBundler.cpp
index 77c89356bc76bb..b68b3c2d5df35a 100644
--- a/clang/lib/Driver/OffloadBundler.cpp
+++ b/clang/lib/Driver/OffloadBundler.cpp
@@ -1010,11 +1010,17 @@ 
CompressedOffloadBundle::compress(llvm::compression::Params P,
 
   uint16_t CompressionMethod = static_cast(P.format);
   uint32_t UncompressedSize = Input.getBuffer().size();
+  uint32_t TotalFileSize = MagicNumber.size() + sizeof(TotalFileSize) +
+   sizeof(Version) + sizeof(CompressionMethod) +
+   sizeof(UncompressedSize) + sizeof(TruncatedHash) +
+   CompressedBuffer.size();
 
   SmallVector FinalBuffer;
   llvm::raw_svector_ostream OS(FinalBuffer);
   OS << MagicNumber;
   OS.write(reinterpret_cast(), sizeof(Version));
+  OS.write(reinterpret_cast(),
+   sizeof(TotalFileSize));
   OS.write(reinterpret_cast(),
sizeof(CompressionMethod));
   OS.write(reinterpret_cast(),
@@ -1034,6 +1040,8 @@ 
CompressedOffloadBundle::compress(llvm::compression::Params P,
 (UncompressedSize / (1024.0 * 1024.0)) / CompressionTimeSeconds;
 
 llvm::errs() << "Compressed bundle format version: " << 

[clang] [ClangOffloadBundler] Add file size to header (PR #88827)

2024-04-15 Thread Yaxun Liu via cfe-commits

yxsamliu wrote:

> Isn't this ABI breaking since we're changing the size of the struct? 
> Shouldn't that necessitate a new version?
> 
> Also unrelated, I wonder if there's a future where we can use the 
> ClangOffloadPackager format in the HIP runtime.

I think you are right. Although this feature has not been used extensively, it 
is better to bump up the version to keep backward compatibility.

Currently we are moving the extraction of code object from fat binary from 
runtime to comgr. We can treat the offload packager format as another flavor of 
fat binary. We only need to pass a pointer to sufficient information to 
__hipRegisterFatBinary, and let runtime pass that pointer to comgr, then comgr 
do the extraction. comgr just needs to identify it is offload packager format 
and handle it properly.

https://github.com/llvm/llvm-project/pull/88827
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [ClangOffloadBundler] Add file size to header (PR #88827)

2024-04-15 Thread Joseph Huber via cfe-commits

https://github.com/jhuber6 commented:

Isn't this ABI breaking since we're changing the size of the struct? Shouldn't 
that necessitate a new version?

Also unrelated, I wonder if there's a future where we can use the 
ClangOffloadPackager format in the HIP runtime.

https://github.com/llvm/llvm-project/pull/88827
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [ClangOffloadBundler] Add file size to header (PR #88827)

2024-04-15 Thread Yaxun Liu via cfe-commits

https://github.com/yxsamliu updated 
https://github.com/llvm/llvm-project/pull/88827

>From c89b262fc8283d985ac31966de73764aedd140af Mon Sep 17 00:00:00 2001
From: "Yaxun (Sam) Liu" 
Date: Mon, 15 Apr 2024 18:02:42 -0400
Subject: [PATCH] [ClangOffloadBundler] Add file size to header

__hipRegisterFatBinary only accepts one pointer argument. It is
expected to get the fat binary size from the header.

This patch adds a file size field to the header of the compressed
bundle.
---
 clang/docs/ClangOffloadBundler.rst|  3 ++
 clang/include/clang/Driver/OffloadBundler.h   | 12 +++---
 clang/lib/Driver/OffloadBundler.cpp   | 41 +--
 .../test/Driver/clang-offload-bundler-zstd.c  | 17 
 4 files changed, 47 insertions(+), 26 deletions(-)

diff --git a/clang/docs/ClangOffloadBundler.rst 
b/clang/docs/ClangOffloadBundler.rst
index 1f8c85a08f8c79..40ffc0176f9b57 100644
--- a/clang/docs/ClangOffloadBundler.rst
+++ b/clang/docs/ClangOffloadBundler.rst
@@ -517,6 +517,9 @@ The compressed offload bundle begins with a header followed 
by the compressed bi
 - **Magic Number (4 bytes)**:
 This is a unique identifier to distinguish compressed offload bundles. The 
value is the string 'CCOB' (Compressed Clang Offload Bundle).
 
+- **Total File Size (32-bit unsigned int)**:
+This is the total size (in bytes) of the file, including the header.
+
 - **Version Number (16-bit unsigned int)**:
 This denotes the version of the compressed offload bundle format. The 
current version is `1`.
 
diff --git a/clang/include/clang/Driver/OffloadBundler.h 
b/clang/include/clang/Driver/OffloadBundler.h
index 65d33bfbd2825f..a39464472d1219 100644
--- a/clang/include/clang/Driver/OffloadBundler.h
+++ b/clang/include/clang/Driver/OffloadBundler.h
@@ -97,6 +97,7 @@ struct OffloadTargetInfo {
 //
 // The format is as follows:
 // - Magic Number (4 bytes) - A constant "CCOB".
+// - Total file size (4 bytes).
 // - Version (2 bytes)
 // - Compression Method (2 bytes) - Uses the values from
 // llvm::compression::Format.
@@ -107,13 +108,14 @@ struct OffloadTargetInfo {
 class CompressedOffloadBundle {
 private:
   static inline const size_t MagicSize = 4;
+  static inline const size_t FileSizeFieldSize = sizeof(uint32_t);
   static inline const size_t VersionFieldSize = sizeof(uint16_t);
   static inline const size_t MethodFieldSize = sizeof(uint16_t);
-  static inline const size_t SizeFieldSize = sizeof(uint32_t);
-  static inline const size_t HashFieldSize = 8;
-  static inline const size_t HeaderSize = MagicSize + VersionFieldSize +
-  MethodFieldSize + SizeFieldSize +
-  HashFieldSize;
+  static inline const size_t UncompressedSizeFieldSize = sizeof(uint32_t);
+  static inline const size_t HashFieldSize = sizeof(uint64_t);
+  static inline const size_t HeaderSize =
+  MagicSize + FileSizeFieldSize + VersionFieldSize + MethodFieldSize +
+  UncompressedSizeFieldSize + HashFieldSize;
   static inline const llvm::StringRef MagicNumber = "CCOB";
   static inline const uint16_t Version = 1;
 
diff --git a/clang/lib/Driver/OffloadBundler.cpp 
b/clang/lib/Driver/OffloadBundler.cpp
index 77c89356bc76bb..23bf0c2322367d 100644
--- a/clang/lib/Driver/OffloadBundler.cpp
+++ b/clang/lib/Driver/OffloadBundler.cpp
@@ -1010,10 +1010,16 @@ 
CompressedOffloadBundle::compress(llvm::compression::Params P,
 
   uint16_t CompressionMethod = static_cast(P.format);
   uint32_t UncompressedSize = Input.getBuffer().size();
+  uint32_t TotalFileSize = MagicNumber.size() + sizeof(TotalFileSize) +
+   sizeof(Version) + sizeof(CompressionMethod) +
+   sizeof(UncompressedSize) + sizeof(TruncatedHash) +
+   CompressedBuffer.size();
 
   SmallVector FinalBuffer;
   llvm::raw_svector_ostream OS(FinalBuffer);
   OS << MagicNumber;
+  OS.write(reinterpret_cast(),
+   sizeof(TotalFileSize));
   OS.write(reinterpret_cast(), sizeof(Version));
   OS.write(reinterpret_cast(),
sizeof(CompressionMethod));
@@ -1033,7 +1039,9 @@ 
CompressedOffloadBundle::compress(llvm::compression::Params P,
 double CompressionSpeedMBs =
 (UncompressedSize / (1024.0 * 1024.0)) / CompressionTimeSeconds;
 
-llvm::errs() << "Compressed bundle format version: " << Version << "\n"
+llvm::errs() << "Total file size (including headers): "
+ << formatWithCommas(TotalFileSize) << " bytes\n"
+ << "Compressed bundle format version: " << Version << "\n"
  << "Compression method used: " << MethodUsed << "\n"
  << "Compression level: " << P.level << "\n"
  << "Binary size before compression: "
@@ -1069,21 +1077,26 @@ CompressedOffloadBundle::decompress(const 
llvm::MemoryBuffer ,
 return llvm::MemoryBuffer::getMemBufferCopy(Blob);
   }
 
+  size_t CurrentOffset = MagicSize;
+  uint32_t 

[clang] [ClangOffloadBundler] Add file size to header (PR #88827)

2024-04-15 Thread via cfe-commits

llvmbot wrote:



@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Yaxun (Sam) Liu (yxsamliu)


Changes

__hipRegisterFatBinary only accepts one pointer argument. It is expected to get 
the fat binary size from the header.

This patch adds a file size field to the header of the compressed bundle.

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


3 Files Affected:

- (modified) clang/include/clang/Driver/OffloadBundler.h (+7-5) 
- (modified) clang/lib/Driver/OffloadBundler.cpp (+28-13) 
- (modified) clang/test/Driver/clang-offload-bundler-zstd.c (+9-8) 


``diff
diff --git a/clang/include/clang/Driver/OffloadBundler.h 
b/clang/include/clang/Driver/OffloadBundler.h
index 65d33bfbd2825f..a39464472d1219 100644
--- a/clang/include/clang/Driver/OffloadBundler.h
+++ b/clang/include/clang/Driver/OffloadBundler.h
@@ -97,6 +97,7 @@ struct OffloadTargetInfo {
 //
 // The format is as follows:
 // - Magic Number (4 bytes) - A constant "CCOB".
+// - Total file size (4 bytes).
 // - Version (2 bytes)
 // - Compression Method (2 bytes) - Uses the values from
 // llvm::compression::Format.
@@ -107,13 +108,14 @@ struct OffloadTargetInfo {
 class CompressedOffloadBundle {
 private:
   static inline const size_t MagicSize = 4;
+  static inline const size_t FileSizeFieldSize = sizeof(uint32_t);
   static inline const size_t VersionFieldSize = sizeof(uint16_t);
   static inline const size_t MethodFieldSize = sizeof(uint16_t);
-  static inline const size_t SizeFieldSize = sizeof(uint32_t);
-  static inline const size_t HashFieldSize = 8;
-  static inline const size_t HeaderSize = MagicSize + VersionFieldSize +
-  MethodFieldSize + SizeFieldSize +
-  HashFieldSize;
+  static inline const size_t UncompressedSizeFieldSize = sizeof(uint32_t);
+  static inline const size_t HashFieldSize = sizeof(uint64_t);
+  static inline const size_t HeaderSize =
+  MagicSize + FileSizeFieldSize + VersionFieldSize + MethodFieldSize +
+  UncompressedSizeFieldSize + HashFieldSize;
   static inline const llvm::StringRef MagicNumber = "CCOB";
   static inline const uint16_t Version = 1;
 
diff --git a/clang/lib/Driver/OffloadBundler.cpp 
b/clang/lib/Driver/OffloadBundler.cpp
index 77c89356bc76bb..23bf0c2322367d 100644
--- a/clang/lib/Driver/OffloadBundler.cpp
+++ b/clang/lib/Driver/OffloadBundler.cpp
@@ -1010,10 +1010,16 @@ 
CompressedOffloadBundle::compress(llvm::compression::Params P,
 
   uint16_t CompressionMethod = static_cast(P.format);
   uint32_t UncompressedSize = Input.getBuffer().size();
+  uint32_t TotalFileSize = MagicNumber.size() + sizeof(TotalFileSize) +
+   sizeof(Version) + sizeof(CompressionMethod) +
+   sizeof(UncompressedSize) + sizeof(TruncatedHash) +
+   CompressedBuffer.size();
 
   SmallVector FinalBuffer;
   llvm::raw_svector_ostream OS(FinalBuffer);
   OS << MagicNumber;
+  OS.write(reinterpret_cast(),
+   sizeof(TotalFileSize));
   OS.write(reinterpret_cast(), sizeof(Version));
   OS.write(reinterpret_cast(),
sizeof(CompressionMethod));
@@ -1033,7 +1039,9 @@ 
CompressedOffloadBundle::compress(llvm::compression::Params P,
 double CompressionSpeedMBs =
 (UncompressedSize / (1024.0 * 1024.0)) / CompressionTimeSeconds;
 
-llvm::errs() << "Compressed bundle format version: " << Version << "\n"
+llvm::errs() << "Total file size (including headers): "
+ << formatWithCommas(TotalFileSize) << " bytes\n"
+ << "Compressed bundle format version: " << Version << "\n"
  << "Compression method used: " << MethodUsed << "\n"
  << "Compression level: " << P.level << "\n"
  << "Binary size before compression: "
@@ -1069,21 +1077,26 @@ CompressedOffloadBundle::decompress(const 
llvm::MemoryBuffer ,
 return llvm::MemoryBuffer::getMemBufferCopy(Blob);
   }
 
+  size_t CurrentOffset = MagicSize;
+  uint32_t TotalFileSize;
+  memcpy(, Blob.data() + CurrentOffset, sizeof(uint32_t));
+  CurrentOffset += FileSizeFieldSize;
+
   uint16_t ThisVersion;
+  memcpy(, Blob.data() + CurrentOffset, sizeof(uint16_t));
+  CurrentOffset += VersionFieldSize;
+
   uint16_t CompressionMethod;
+  memcpy(, Blob.data() + CurrentOffset, sizeof(uint16_t));
+  CurrentOffset += MethodFieldSize;
+
   uint32_t UncompressedSize;
+  memcpy(, Blob.data() + CurrentOffset, sizeof(uint32_t));
+  CurrentOffset += UncompressedSizeFieldSize;
+
   uint64_t StoredHash;
-  memcpy(, Input.getBuffer().data() + MagicNumber.size(),
- sizeof(uint16_t));
-  memcpy(, Blob.data() + MagicSize + VersionFieldSize,
- sizeof(uint16_t));
-  memcpy(,
- Blob.data() + MagicSize + VersionFieldSize + MethodFieldSize,
- sizeof(uint32_t));
-  memcpy(,
- Blob.data() + MagicSize + VersionFieldSize + MethodFieldSize +
- 

[clang] [ClangOffloadBundler] Add file size to header (PR #88827)

2024-04-15 Thread Yaxun Liu via cfe-commits

https://github.com/yxsamliu created 
https://github.com/llvm/llvm-project/pull/88827

__hipRegisterFatBinary only accepts one pointer argument. It is expected to get 
the fat binary size from the header.

This patch adds a file size field to the header of the compressed bundle.

>From 3deb7005c46238a975a5f42e1e4a47d4457ea9ac Mon Sep 17 00:00:00 2001
From: "Yaxun (Sam) Liu" 
Date: Mon, 15 Apr 2024 18:02:42 -0400
Subject: [PATCH] [ClangOffloadBundler] Add file size to header

__hipRegisterFatBinary only accepts one pointer argument. It is
expected to get the fat binary size from the header.

This patch adds a file size field to the header of the compressed
bundle.
---
 clang/include/clang/Driver/OffloadBundler.h   | 12 +++---
 clang/lib/Driver/OffloadBundler.cpp   | 41 +--
 .../test/Driver/clang-offload-bundler-zstd.c  | 17 
 3 files changed, 44 insertions(+), 26 deletions(-)

diff --git a/clang/include/clang/Driver/OffloadBundler.h 
b/clang/include/clang/Driver/OffloadBundler.h
index 65d33bfbd2825f..a39464472d1219 100644
--- a/clang/include/clang/Driver/OffloadBundler.h
+++ b/clang/include/clang/Driver/OffloadBundler.h
@@ -97,6 +97,7 @@ struct OffloadTargetInfo {
 //
 // The format is as follows:
 // - Magic Number (4 bytes) - A constant "CCOB".
+// - Total file size (4 bytes).
 // - Version (2 bytes)
 // - Compression Method (2 bytes) - Uses the values from
 // llvm::compression::Format.
@@ -107,13 +108,14 @@ struct OffloadTargetInfo {
 class CompressedOffloadBundle {
 private:
   static inline const size_t MagicSize = 4;
+  static inline const size_t FileSizeFieldSize = sizeof(uint32_t);
   static inline const size_t VersionFieldSize = sizeof(uint16_t);
   static inline const size_t MethodFieldSize = sizeof(uint16_t);
-  static inline const size_t SizeFieldSize = sizeof(uint32_t);
-  static inline const size_t HashFieldSize = 8;
-  static inline const size_t HeaderSize = MagicSize + VersionFieldSize +
-  MethodFieldSize + SizeFieldSize +
-  HashFieldSize;
+  static inline const size_t UncompressedSizeFieldSize = sizeof(uint32_t);
+  static inline const size_t HashFieldSize = sizeof(uint64_t);
+  static inline const size_t HeaderSize =
+  MagicSize + FileSizeFieldSize + VersionFieldSize + MethodFieldSize +
+  UncompressedSizeFieldSize + HashFieldSize;
   static inline const llvm::StringRef MagicNumber = "CCOB";
   static inline const uint16_t Version = 1;
 
diff --git a/clang/lib/Driver/OffloadBundler.cpp 
b/clang/lib/Driver/OffloadBundler.cpp
index 77c89356bc76bb..23bf0c2322367d 100644
--- a/clang/lib/Driver/OffloadBundler.cpp
+++ b/clang/lib/Driver/OffloadBundler.cpp
@@ -1010,10 +1010,16 @@ 
CompressedOffloadBundle::compress(llvm::compression::Params P,
 
   uint16_t CompressionMethod = static_cast(P.format);
   uint32_t UncompressedSize = Input.getBuffer().size();
+  uint32_t TotalFileSize = MagicNumber.size() + sizeof(TotalFileSize) +
+   sizeof(Version) + sizeof(CompressionMethod) +
+   sizeof(UncompressedSize) + sizeof(TruncatedHash) +
+   CompressedBuffer.size();
 
   SmallVector FinalBuffer;
   llvm::raw_svector_ostream OS(FinalBuffer);
   OS << MagicNumber;
+  OS.write(reinterpret_cast(),
+   sizeof(TotalFileSize));
   OS.write(reinterpret_cast(), sizeof(Version));
   OS.write(reinterpret_cast(),
sizeof(CompressionMethod));
@@ -1033,7 +1039,9 @@ 
CompressedOffloadBundle::compress(llvm::compression::Params P,
 double CompressionSpeedMBs =
 (UncompressedSize / (1024.0 * 1024.0)) / CompressionTimeSeconds;
 
-llvm::errs() << "Compressed bundle format version: " << Version << "\n"
+llvm::errs() << "Total file size (including headers): "
+ << formatWithCommas(TotalFileSize) << " bytes\n"
+ << "Compressed bundle format version: " << Version << "\n"
  << "Compression method used: " << MethodUsed << "\n"
  << "Compression level: " << P.level << "\n"
  << "Binary size before compression: "
@@ -1069,21 +1077,26 @@ CompressedOffloadBundle::decompress(const 
llvm::MemoryBuffer ,
 return llvm::MemoryBuffer::getMemBufferCopy(Blob);
   }
 
+  size_t CurrentOffset = MagicSize;
+  uint32_t TotalFileSize;
+  memcpy(, Blob.data() + CurrentOffset, sizeof(uint32_t));
+  CurrentOffset += FileSizeFieldSize;
+
   uint16_t ThisVersion;
+  memcpy(, Blob.data() + CurrentOffset, sizeof(uint16_t));
+  CurrentOffset += VersionFieldSize;
+
   uint16_t CompressionMethod;
+  memcpy(, Blob.data() + CurrentOffset, sizeof(uint16_t));
+  CurrentOffset += MethodFieldSize;
+
   uint32_t UncompressedSize;
+  memcpy(, Blob.data() + CurrentOffset, sizeof(uint32_t));
+  CurrentOffset += UncompressedSizeFieldSize;
+
   uint64_t StoredHash;
-  memcpy(, Input.getBuffer().data() + MagicNumber.size(),
- sizeof(uint16_t));
-