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

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

Can we test this, or is it tested with a follow up commit at least? If so, which one (add to commit message as well, e.g., tests contained in D...).
Some notes, exclusively on the unused do not strip flag. Everything else looks good reading it.



================
Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:188
+    return createFileError(TempFile, EC);
+
+  SmallVector<StringRef, 8> StripArgs;
----------------
```
if (!StripSections)
  return static_cast<std::string>(TempFile);
```


================
Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:258
+  if (ToBeDeleted.empty())
+    return None;
+
----------------
if (!StripSections)
  return None;


================
Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:435
+        Arg = **NewFileOrErr;
+      }
+    }
----------------
Does this work with the "do not strip option"? I somehow doubt it and would expect an error.


================
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";
 
----------------
I know this is not new but I don't understand the warning here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116545