[PATCH] D68746: [Clang][OpenMP Offload] Move offload registration code to the wrapper

Alexey Bataev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Oct 10 07:35:18 PDT 2019


ABataev added inline comments.


================
Comment at: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp:74
+  IntegerType *getSizeTTy() {
+    switch (M.getDataLayout().getPointerTypeSize(Type::getInt8PtrTy(C))) {
+    case 4u:
----------------
Same question as before: maybe better to make the size of size_t type a parameter of a tool?


================
Comment at: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp:174
+  /// Global variable that represents BinDesc is returned.
+  GlobalVariable *createBinDesc(ArrayRef<ArrayRef<char>> Bufs) {
+    // Create external begin/end symbols for the offload entries table.
----------------
Why `ArrayRef<char>`, not `StringRef`? It has a constructor for pointer/length pair.


================
Comment at: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp:204
+    ImagesInits.reserve(Bufs.size());
+    for (const ArrayRef<char> &Buf : Bufs) {
+      auto *Data = ConstantDataArray::get(C, Buf);
----------------
No need for `const` and `&` here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68746/new/

https://reviews.llvm.org/D68746





More information about the cfe-commits mailing list