[PATCH] D64943: [Clang][OpenMP offload] Eliminate use of OpenMP linker script

Alexey Bataev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Aug 16 07:44:27 PDT 2019

ABataev added a comment.

In D64943#1619958 <https://reviews.llvm.org/D64943#1619958>, @sdmitriev wrote:

> As I understand ‘atexit’ solution would be target dependent (‘__cxa_atexit’ on Linux and ‘atexit’ on Windows) whereas @llvm.global_ctors/dtors variables offer similar and platform neutral functionality (http://llvm.org/docs/LangRef.html#the-llvm-global-ctors-global-variable). Why do you think that ‘atexit’ is a better choice?

Because it does not work on Windows, better to have portable solution, if possible.

Comment at: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp:72
+  IntegerType *getSizeTTy() {
+    switch (M.getDataLayout().getPointerTypeSize(Type::getInt8PtrTy(C))) {
1. Markt it `const`.
2. This still is not the best solution, since `size_t` not necessarily has the pointer size. I don't know if there is a better solution. @hfinkel? If this is the best, why not just to use `getIntPtrType(C)`?

Comment at: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp:135
+  // Creates binary descriptor for the given device images. Binary descriptor is
+  // an object that is passed to the offloading runtime at program startup and
Use `\\\` style for comments here

Comment at: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp:171
+  // Global variable that represents BinDesc is returned.
+  GlobalVariable *createBinDesc(const SmallVectorImpl<MemoryBufferRef> &Bufs) {
+    // Create external begin/end symbols for the offload entries table.
Use `ArrayRef` instead of `const SmallVectorImpl &`

Comment at: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp:330
+  BufferRefs.reserve(Inputs.size());
+  for (const auto &File : Inputs) {
+    auto BufOrErr = MemoryBuffer::getFileOrSTDIN(File);
Use `std:string` instead of `auto`

Comment at: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp:331
+  for (const auto &File : Inputs) {
+    auto BufOrErr = MemoryBuffer::getFileOrSTDIN(File);
+    if (!BufOrErr) {
Also, better to specify type expplicitly.



More information about the cfe-commits mailing list