tra added a comment.
Syntax/style looks OK to me with a few nits.
================
Comment at: clang/test/Driver/linker-wrapper.c:109
// RUN: clang-offload-packager -o %t-lib.out \
// RUN:
--image=file=%S/Inputs/dummy-elf.o,kind=openmp,triple=nvptx64-nvidia-cuda,arch=sm_70
\
----------------
Nit: This test case does not have any CHECK lines and could use a comment
describing what it's supposed to test. AFAICT it's intended to make sure that
no temporary files are left around, but I'm not 100% sure.
================
Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:620-622
+ CmdArgs.push_back("-bundle-align=4096");
+
+ SmallVector<StringRef> Targets = {"-targets=host-x86_64-unknown-linux"};
----------------
We probably do not want to hardcode the assumption that the host is x86_64
linux.
Bundle alignment should probably also be target-dependent, but 4K is common
enough and is probably fine in practice.
================
Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:1205
+bundleHIP(ArrayRef<OffloadingImage> Images) {
+ SmallVector<std::unique_ptr<MemoryBuffer>> Buffers;
+
----------------
I'd move it to the end where the buffer is actually used.
================
Comment at: clang/tools/clang-linker-wrapper/OffloadWrapper.cpp:393
+ /*Initializer*/ nullptr,
+ IsHIP ? "__start_hip_offloading_entries"
+ : "__start_cuda_offloading_entries");
----------------
We should probably have a helper function returning properly prefixed name,
similar to what we do in clang:
https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/CGCUDANV.cpp#L184
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128914/new/
https://reviews.llvm.org/D128914
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
