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 +private: + 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. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64943/new/ https://reviews.llvm.org/D64943 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
