[PATCH] D93525: [OpenMP] Add unbundling of archives containing bundled object files into device specific archives

Alexey Bataev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Dec 18 08:49:01 PST 2020


ABataev added inline comments.


================
Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:142-144
+  } else {
+    return Triple;
+  }
----------------
No need `else` here


================
Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:953-957
+  else {
+    WithColor::warning() << "Could not determine extension for archive "
+                            "members, using \".o\"\n";
+    return ".o";
+  }
----------------
No need for `else` here


================
Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:960-962
+static StringRef removeExtension(StringRef FileName) {
+  return (FileName.contains(".")) ? FileName.rsplit('.').first : FileName;
+}
----------------
I think llvm Support lib has all required functions for this.


================
Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:1000
+  ArchiveBuffers.push_back(std::move(*BufOrErr));
+  auto LibOrErr = Archive::create(ArchiveBuffers.back()->getMemBufferRef());
+  if (!LibOrErr)
----------------
Do not use `auto` where the type is not obvious.


================
Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:1048-1049
+    while (!CurKindTriple.empty()) {
+      if (hasHostKind(CurKindTriple)) {
+        // Do nothing, we don't extract host code yet
+      } else if (checkDeviceOptions(getDevice(getTriple(CurKindTriple)),
----------------
Just `continue` and make `else if` just `if`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93525



More information about the cfe-commits mailing list