Hahnfeld accepted this revision.
Hahnfeld added a reviewer: Hahnfeld.
Hahnfeld added a comment.
This revision is now accepted and ready to land.
LGTM with only some minor nits on some of the comments and a CMake question
================
Comment at: test/CMakeLists.txt:27-33
@@ -26,8 +26,9 @@
list(APPEND CLANG_TEST_DEPS
clang clang-headers
clang-format
c-index-test diagtool
clang-tblgen
+ clang-offload-bundler
)
----------------
I think `clang-offload-bundler` needs to be added as dependency for the `clang`
target because it will really need the bundler at runtime, not only when
testing...
(Disclaimer: I'm no CMake expert)
================
Comment at: tools/clang-offload-bundler/ClangOffloadBundler.cpp:102-104
@@ +101,5 @@
+ virtual void ReadHeader(MemoryBuffer &Input) = 0;
+ /// Read the marker of the next bundled to be read in the file. The triple of
+ /// the target associated with that bundled is returned. An empty string is
+ /// returned if there are no more bundles to be read.
+ virtual StringRef ReadBundleStart(MemoryBuffer &Input) = 0;
----------------
s/bundled/bundle/?
================
Comment at: tools/clang-offload-bundler/ClangOffloadBundler.cpp:151
@@ +150,3 @@
+
+/// Read 8-byte integers to/from a buffer in little-endian format.
+static uint64_t Read8byteIntegerFromBuffer(StringRef Buffer, size_t pos) {
----------------
`to/from`?
================
Comment at: tools/clang-offload-bundler/ClangOffloadBundler.cpp:164
@@ +163,3 @@
+
+/// Write and write 8-byte integers to/from a buffer in little-endian format.
+static void Write8byteIntegerToBuffer(raw_fd_ostream &OS, uint64_t Val) {
----------------
Duplicate `and write`? `to/from`?
================
Comment at: tools/clang-offload-bundler/ClangOffloadBundler.cpp:568
@@ +567,3 @@
+ if (!FoundHostBundle) {
+ llvm::errs() << "error: Can't find bundles for all requested targets\n";
+ return true;
----------------
Better say that we haven't found the bundle for the host?
https://reviews.llvm.org/D13909
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
