[PATCH] D116545: [OpenMP] Add support for extracting device code in linker wrapper

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added inline comments.


================
Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:258
+  if (ToBeDeleted.empty())
+    return None;
+
----------------
jdoerfert wrote:
> if (!StripSections)
>   return None;
Fixed this later, I could rebase it so it applies here if it makes the review easier.


================
Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:435
+        Arg = **NewFileOrErr;
+      }
+    }
----------------
jdoerfert wrote:
> Does this work with the "do not strip option"? I somehow doubt it and would expect an error.
If we don't strip then we just return `None` to indicate we didn't create a new host file and this code won't execute.


================
Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:445
   // TODO: Wrap device image in a host binary and pass it to the linker.
   WithColor::warning(errs(), argv[0]) << "Offload linking not yet supported.\n";
 
----------------
jdoerfert wrote:
> I know this is not new but I don't understand the warning here.
it was just a stand-in to indicate that the tool was indeed running with the `-fopenmp-new-driver` flag, but wasn't doing the offload linking step and thus wouldn't execute on the device. It's removed once offloading actually works.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116545