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

[Sergey Dmitriev via Phabricator via cfe-commits]

sdmitriev 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
----------------
ABataev wrote:
> 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?
1. Will fix.
2. Ok.
3. I do not think that anybody is working on that problem now, so it is not clear how much time that will take. Also merging two llvm-objcopy invocations into one will be a simple NFC change, so I think this should not block this patch.


================
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);
----------------
ABataev wrote:
> from
This was supposed to be 'for'.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73408