tra added inline comments.
================
Comment at: clang/docs/ClangOffloadBinary.rst:8
+
+.. _clang-offload-binary:
+
----------------
Naming nit: `binary` may not be the best term for what we're trying to do here.
Perhaps something like `package`, `container` or `collection` would work better.
================
Comment at: clang/docs/ClangOffloadBinary.rst:42
+
+ clang-offload-binary options:
+
----------------
This appears to be a one-way process. How one would examine what's in the
binary and unpack/extract specific component from it?
================
Comment at: clang/test/Frontend/embed-object.ll:7
+; CHECK: @[[OBJECT_1:.+]] = private constant [0 x i8] zeroinitializer, section
".llvm.offloading", align 8
+; CHECK: @[[OBJECT_2:.+]] = private constant [0 x i8] zeroinitializer, section
".llvm.offloading", align 8
; CHECK: @llvm.compiler.used = appending global [3 x ptr] [ptr @x, ptr
@[[OBJECT_1]], ptr @[[OBJECT_2]]], section "llvm.metadata"
----------------
What will happen if an openMP file compiled this way is linked with the older
version of OpenMP runtime which presumably expected to see extra data in
`.llvm.offloading`?
Will it provide a sensible error? Perhaps we should change the section name,
too.
================
Comment at: clang/tools/clang-offload-binary/ClangOffloadBinary.cpp:70
+ raw_svector_ostream OS(BinaryData);
+ for (StringRef Image : DeviceImages) {
+ StringMap<StringRef> Args;
----------------
It would be useful to add a comment describing the 'special' keys `file` and
`kind`.
================
Comment at: clang/tools/clang-offload-binary/ClangOffloadBinary.cpp:75
+
+ if (!Args.count("triple") || !Args.count("file"))
+ return reportError(createStringError(
----------------
Should `kind` also be required? If not, what's the default kind?
================
Comment at: clang/tools/clang-offload-binary/ClangOffloadBinary.cpp:99
+ }
+ std::unique_ptr<MemoryBuffer> Buffer = OffloadBinary::write(ImageBinary);
+ OS << Buffer->getBuffer();
----------------
Nit: `write` is a rather misleading function name here. AFAICT, we're not
actually writing anything, but rather packing the `ImageBinary` into a memory
buffer, which we then return.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125165/new/
https://reviews.llvm.org/D125165
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
