[PATCH] D78988: [LTO] Suppress emission of the empty object file

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

I know you said you needed to fix a few test failures, but I have some comments/questions on the first version of the patch anyway.



================
Comment at: llvm/include/llvm/LTO/Config.h:71
+  /// build system which want to know a priori all possible output files.
+  bool EmitEmptyObj = false;
+
----------------
Since this only applies to the regular LTO object, would be better to call this something more specific, like AlwaysEmitRegularLTOObj.


================
Comment at: llvm/lib/LTO/LTOBackend.cpp:503
+        Mod->getModuleInlineAsm() == "" && !C.EmitEmptyObj)
+      HookOnly = true;
+    codegen(C, TM.get(), AddStream, 0, *Mod, HookOnly);
----------------
Just to clarify my assumption, you are doing this here rather than down in codegen because for ThinLTO modules we want to produce an object even when an empty module is explicitly linked for some reason?

Rather than that series of checks, I think it would be better to just include a flag on the RegularLTOState to indicate whether addRegularLTO was ever called. It's clearer and probably less fragile.

Also - what is the value of calling the hook if it is an empty default module that wasn't added by the link, and that we aren't going to code gen?


================
Comment at: llvm/tools/gold/gold-plugin.cpp:1058
   size_t MaxTasks = Lto->getMaxTasks();
-  std::vector<std::pair<SmallString<128>, bool>> Files(MaxTasks);
-
-  auto AddStream =
-      [&](size_t Task) -> std::unique_ptr<lto::NativeObjectStream> {
-    Files[Task].second = !SaveTemps;
-    int FD = getOutputFileName(Filename, /* TempOutFile */ !SaveTemps,
-                               Files[Task].first, Task);
-    return std::make_unique<lto::NativeObjectStream>(
-        std::make_unique<llvm::raw_fd_ostream>(FD, true));
-  };
+  std::vector<SmallString<0>> Buf(MaxTasks);
+  std::vector<std::unique_ptr<MemoryBuffer>> CacheFiles(MaxTasks);
----------------
Why are all these changes needed, since you are addressing this within LTO to not emit the empty object file?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78988