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

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/include/llvm/LTO/LTO.h:198
+  }
+  NativeObjectStream(raw_pwrite_stream *OS) : OS(OS) {}
+  /// NativeObjectStream may own the allocation of the used raw_pwrite_stream,
----------------
dexonsmith wrote:
> As a drive-by comment: if this sticks around, please make construct this from a reference to prevent raw owned pointers from coming in this way (if we need a constructor from `nullptr` the unique_ptr constructor will still work). (That said, I agree with Steven that changing the test tool would be better.)
Thanks, the code is gone now


================
Comment at: llvm/include/llvm/LTO/LTO.h:202
+  std::unique_ptr<raw_pwrite_stream> OS_;
+  raw_pwrite_stream *OS;
   virtual ~NativeObjectStream() = default;
----------------
steven_wu wrote:
> I guess this only exists to support `bool compileOptimized(ArrayRef<raw_pwrite_stream *> Out)`?
> 
> That seems to only an interface used in `llvm-lto` which is a test tool. I wonder if we can just change the interface so we don't need to worry about if NativeObjectStream owning the output stream or not.
> I guess this only exists to support bool compileOptimized(ArrayRef<raw_pwrite_stream *> Out)?

There's a another users in LTOCodegenerator, but that can also be updated. I changed to interface to accept and `AddStream` callback and the parallelism level. This code is not longer needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94487