[PATCH] D33525: [ThinLTO] Migrate ThinLTOBitcodeWriter to the new PM.

Chandler Carruth via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu May 25 02:28:37 PDT 2017


chandlerc added a comment.

(focusing on the LLVM side of this review for now)

Can you add an LLVM-based test? Can you add this to `lib/Passes/PassRegistry.def`?



================
Comment at: llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp:423
+
+class AARGetter {
+  FunctionAnalysisManager &AM;
----------------
timshen wrote:
> mehdi_amini wrote:
> > Can't you do it with a lambda?
> I can, except that AAR needs to be allocated outside of the lambda, on the stack:
> 
>     Optional<AAResults> AAR;
>     auto F = [&AAR](Function &) -> AAResults& { ... };
> 
> (AAR can't be captured by value, since AAResults is not copyable) this way AAR out-lives the lambda, and I feel less readable.
You don't need to keep a copy though... You can directly expose the reference returned from the `FunctionAnalysisManager` -- that reference will stay alive until the result is invalidated. So this will become something more like:

  [&FAM](Function &F) { return FAM.getResult<AAManager>(F); }


https://reviews.llvm.org/D33525





More information about the llvm-commits mailing list