[PATCH] D26832: [LTO] Add option to generate optimization records

[Davide Italiano via llvm-commits]

davide accepted this revision.
davide added a reviewer: davide.
davide added a comment.
This revision is now accepted and ready to land.

LGTM with the remaining comment fixed but please wait for a second opinion.



================
Comment at: lib/LTO/LTOCodeGenerator.cpp:96-100
+static cl::opt<std::string>
+    RemarksFilename("pass-remarks-output",
+                    cl::desc("Output filename for pass remarks"),
+                    cl::value_desc("filename"));
+
----------------
anemet wrote:
> davide wrote:
> > Side note, I very much dislike adding other `cl::opt` in library code, but this is what we have now. If I recall correctly, @beanz has plans for moving away of them, but not sure how far away it is.
> > If you use the new `lib/LTO` interface maybe you will be able to get rid of it (but you should use `llvm-lto2` to test.
> Does the Darwin linker support the new interface?
I don't know about ld64 (and I don't think you're talking about lld as it doesn't do LTO on Mach-O).
So, unfortunately, it seems that `cl::opt` is the only viable option.


================
Comment at: lib/LTO/LTOCodeGenerator.cpp:521-523
+    DiagnosticOutputFile->keep();
+    // FIXME: LTOCodeGenerator dtor is not invoked on Darwin
+    DiagnosticOutputFile->os().flush();
----------------
anemet wrote:
> davide wrote:
> > have you already diagnosed why?
> I *think* that's a known bug in our linker.  Mehdi can correct me if I am wrong.
Fair enough.


https://reviews.llvm.org/D26832