[PATCH] D14228: [FunctionAttrs] Identify norecurse functions

[James Molloy via llvm-commits]

Hi Manman, Mehdi,

Yes, I agree. The implementation in FunctionAttrs is more conservative than
it could be here (for simplicity) - I will change the comment to make that
explicit.

Cheers,

James

On Wed, 4 Nov 2015 at 21:30 Manman Ren via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> manmanren added inline comments.
>
> ================
> Comment at: lib/Transforms/IPO/FunctionAttrs.cpp:1832
> @@ +1831,3 @@
> +  // For a function not to recurse, it must be in its own SCC and must not
> +  // call itself or be callable by external functions.
> +  if (!SCC.isSingular())
> ----------------
> joker.eph wrote:
> > manmanren wrote:
> > > This comment seems to be incorrect. Should it be "must not call itself
> or any function that may be recursive"?
> > >
> > It can call a recursive function, as long at it is not involved in the
> recursion.
> >
> > i.e. if:
> >  a() calls b()
> >  b() calls c()
> >  c() calls b()
> >
> > a is not recursive, but b() and c() are.
> >
> > The issue with external function is that you don't know if they can call
> back into a() or one of its callers.
> > Or did I misunderstand what you meant?
> I was talking about what is actually implemented. But it is not the
> necessary condition.
>
>  // For a function not to recurse, it must be in its own SCC and must not
>   // call itself or any function that may be recursive.
> is incorrect.
>
> But this makes sense:
> // We mark a function as norecurse if it is in its own SCC and it does not
> call itself
> // or any function that may be recursive.
>
>
>
> Repository:
>   rL LLVM
>
> http://reviews.llvm.org/D14228
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151105/9eb03fb9/attachment.html>