[PATCH] D73408: [Clang][Bundler] Add 'exclude' flag to target objects sections

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.


================
Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:467-475
+    // We will use llvm-objcopy to add target objects’ sections to the output
+    // fat object. These sections should have ‘exclude’ flag set which tells
+    // link editor to remove them from linker inputs when linking executable or
+    // shared library. llvm-objcopy currently does not support adding new
+    // section and changing flags for the added section in one invocation, and
+    // because of that we have to run it two times. First run adds sections and
+    // the second changes flags. TODO: change it to one run once llvm-objcopy
----------------
1. Extra spaces in the comment.
2. TODO better to have on a separate line.
3. Maybe it is better to postpone it until llvm-objcopy can do this in one run?


================
Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:508
+
+    auto ExecuteObjcopy = [&Objcopy](ArrayRef<StringRef> ObjcopyArgs) -> Error {
+      // If the user asked for the commands to be printed out, we do that
----------------
Better to make it a function


================
Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:527
+
+    // And run llvm-objcopy fro the second time to update section flags.
+    ObjcopyArgs.resize(1);
----------------
from


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73408