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

[Kuan Hsu Chen (Zakk) via Phabricator via llvm-commits]

khchen marked 6 inline comments as done.
khchen added inline comments.


================
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);
----------------
tejohnson wrote:
> khchen wrote:
> > tejohnson wrote:
> > > Why are all these changes needed, since you are addressing this within LTO to not emit the empty object file?
> > original implementation always generates file including empty module which generate empty file. (not object file)
> > 
> > This implementation reference [[ https://github.com/llvm/llvm-project/blob/master/lld/ELF/LTO.cpp#L274 | lld ]].
> > In https://reviews.llvm.org/D78035#inline-719821 as you mentioned we can "possibly make the Files array a tuple with another bool flag indicating whether the obj file buffer was empty." but I could not find a good place to store empty info when calling lto::backend.
> > 
> > any suggestion?
> > 
> > 
> That was an alternate suggestion, instead of changing LTO, since you mentioned lld worked fine. Since you are suppressing the object file in LTO, is any change needed here? AddStream isn't called until down in codegen(), and with your change that isn't being executed. So presumably no change is needed here to prevent the empty object from being passed back to gold. 
You are right, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78988