[PATCH] D63978: Clang Interface Stubs merger plumbing for Driver

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added a comment.

Tests?



================
Comment at: clang/lib/Driver/Driver.cpp:3420
 
+  // Add a link action if necessary.
+  if (!MergerInputs.empty()) {
----------------
This isn't really a link action ...


================
Comment at: clang/lib/Driver/Driver.cpp:3423
+    Action *LA = C.MakeAction<IfsMergeJobAction>(MergerInputs, types::TY_Image);
+    LA = OffloadBuilder.processHostLinkAction(LA);
+    Actions.push_back(LA);
----------------
I don't understand why the offload bundler is part of the interface merge phase.


================
Comment at: clang/lib/Driver/ToolChains/InterfaceStubs.cpp:11
+#include "CommonArgs.h"
+#include "clang/Config/config.h" // for GCC_INSTALL_PREFIX
+#include "clang/Driver/Compilation.h"
----------------
Why do you need `GCC_INSTALL_PREFIX`?  GCC doesn't have interface stubs yet AFAIK.


================
Comment at: clang/lib/Driver/ToolChains/InterfaceStubs.cpp:34
+                                          const char *LinkingOutput) const {
+  llvm::errs() << "Construct merger job\n";
+  SmallVector<const char *, 16> CmdArgs;
----------------
debugging left overs?


================
Comment at: clang/lib/Driver/ToolChains/InterfaceStubs.cpp:36
+  SmallVector<const char *, 16> CmdArgs;
+  const char *Exec = "/bin/echo";
+  // CmdArgs.push_back("-o");
----------------
Hmm, this seems wrong :-).  You should search for it relative to the toolchain and then fallback.


================
Comment at: clang/lib/Driver/Types.cpp:328
+        IfsModePhaseList, std::back_inserter(P), [&DAL](phases::ID Phase) {
+          return Phase <= ((DAL.getLastArg(options::OPT_c)) ? phases::Compile
+                                                            : phases::IfsMerge);
----------------
How does `-c` `-emit-interface-stubs` and multiple input play together?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63978