[PATCH] D74162: [Inliner] Inlining should honor nobuiltin attributes

Guillaume Chatelet via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 7 07:15:15 PST 2020


gchatelet accepted this revision.
gchatelet marked an inline comment as done.
gchatelet added a comment.
This revision is now accepted and ready to land.

Let's wait a bit for other reviewers to comment.



================
Comment at: llvm/lib/Transforms/IPO/PartialInlining.cpp:393
     return PartialInlinerImpl(&GetAssumptionCache, LookupAssumptionCache,
-                              &GetTTI, NoneType::None, PSI)
+                              &GetTTI, NoneType::None, &GetTLI, PSI)
         .run(M);
----------------
tejohnson wrote:
> gchatelet wrote:
> > I'm having a hard time convincing myself that the lifetime requirements are correct here.
> > Passing a local variable `GetTLI` by address in `return` statement looks fishy.
> > 
> > It's similar to `GetTTI` so is seems correct, it's just hard to tell by looking at the code.
> > 
> > Same above and below.
> What's being returned is the bool result of the run() call, not the PartialInlinerImpl object, which doesn't survive past this function and therefore the GetTLI scope.
Ha right, I got confused by the formatting and missed the `run()` on next line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74162





More information about the llvm-commits mailing list