Meinersbur added a comment.

This patch reduced the build time from 12 to 7 minutes? Sounds too good to be 
true. Thanks for the work! I would primarily interested in this to avoid 
determining the `-cc1` command line when debugging.

However, the implementation of calling it's own main function gives the 
impression to be more like a hack. How much restructuring would be needed to 
properly create a CompilerInstance inside the driver instead?



================
Comment at: clang/lib/Driver/Job.cpp:340-355
+  typedef int (*ClangDriverMainFunc)(SmallVectorImpl<const char *> &);
+  ClangDriverMainFunc ClangDriverMain = nullptr;
+
+  if (ReenterTool) {
+    StringRef F = llvm::sys::path::filename(Executable);
+    if (F.endswith_lower(".exe"))
+      F = F.drop_back(4);
----------------
This looks fragile as it will break when the user chooses to rename the 
executable (`clang-cuda`, `--driver-mode=...`,...). Why not moving the 
ClangDriverMain logic into the clangDriver module. Or, at least, pass it 
through a lambda. It would also make it easier to use clangDriver as a library 
(if that was ever an intended use-case?!?)


================
Comment at: clang/tools/driver/driver.cpp:337
+
+int ClangDriverMain(SmallVectorImpl<const char *> &argv) {
+  static LLVM_THREAD_LOCAL bool EnterPE = true;
----------------
Could you add a comment for the purpose of this function? It's not obvious 
outside the context of this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69825



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to