[PATCH] D94487: [LTO] Use lto::backend for code generation (WIP).

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

In D94487#2492933 <https://reviews.llvm.org/D94487#2492933>, @tejohnson wrote:

> Thanks, I'll take a look hopefully today. As I just mentioned in the bug, ThinLTOCodeGenerator has the same issue - will you be migrating that as well?

Yes, I can do that as well, once `LTOCodeGenerator.cpp` is wrapped up.



================
Comment at: llvm/include/llvm/LTO/LTO.h:199
+  NativeObjectStream(raw_pwrite_stream *OS) : OS(OS) {}
+  std::unique_ptr<raw_pwrite_stream> OS_;
+  raw_pwrite_stream *OS;
----------------
this could do with a comment. The changes here are because the existing uses allocate a new `raw_pwrite_stream` in a closure, and `NativeObjectStream` owns that. but in `LTOCodeGenerator`, we already opened the stream earlier, so we just have a regular pointer.


================
Comment at: llvm/lib/LTO/LTOCodeGenerator.cpp:264
+    auto X = std::make_unique<lto::NativeObjectStream>(&objFile.os());
+    return X;
+  };
----------------
I'll change that to just return.


================
Comment at: llvm/lib/LTO/LTOCodeGenerator.cpp:270
+  if (Error Err =
+          backend(Conf, AddStream, 1, std::move(MergedModule), CombinedIndex))
+    genResult = false;
----------------
tejohnson wrote:
> Note that backend() when not called in CodeGenOnly mode will pretty much just call opt() and codegen(). With some more restructuring you wouldn't need separate calls to opt() from optimize() and backend with CodeGenOnly=1 here, but rather a single call to backend. Maybe just add a FIXME or other note to that effect here? Because it seems like longer term some of this could be cleaned up.
> Because it seems like longer term some of this could be cleaned up.

Agreed, I think the long-term plan is to further unify/simplify the code. But given that the test coverage of the code is relatively limited, I think it would be safest if we move in small, incremental steps, to limit any breakage and make it easier to track down issues.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94487