[PATCH] D15676: [attrs] Extract the pure inference of function attributes into a standalone pass.

James Molloy via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 21 02:07:45 PST 2015


Hi Mehdi,

> For strlen: you’re right that it is not correct in the absolute, but for
what we use the information I’m not sure it matters. We’re interested in
the fact that it won’t call back in the user code in any way, so that we
can infer the norecurse on the callers.

I understand where you're coming from, but I think second-guessing how an
attribute may be used is a dangerous route to go down. "norecurse" could be
used for something else in the future, so unless we have a good reason it
would be good not to make assumptions.

James

On Mon, 21 Dec 2015 at 10:06 James Molloy via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> jmolloy accepted this revision.
> jmolloy added a comment.
> This revision is now accepted and ready to land.
>
> Hi Chandler,
>
> Thanks for taking the time to do this.
>
> I agree with Philip - while I like that you've added extra norecurse
> attributes, I don't like that it's smuggled in as part of this patch. I'd
> highly prefer this patch to be NFC and then a simple update patch adding
> the norecurse attributes.
>
> Apart from that and the specific comment below, it LGTM.
>
> Cheers,
>
> James
>
>
> ================
> Comment at: include/llvm/Transforms/IPO/InferFunctionAttrs.h:25
> @@ +24,3 @@
> +/// A pass which infers function attributes from the names and signatures
> of
> +/// function declarations in a module..
> +class InferFunctionAttrsPass {
> ----------------
> Double period.
>
> ================
> Comment at: include/llvm/Transforms/IPO/InferFunctionAttrs.h:26
> @@ +25,3 @@
> +/// function declarations in a module..
> +class InferFunctionAttrsPass {
> +public:
> ----------------
> Unrelated: Damn, new style passes look so much nicer!
>
>
> http://reviews.llvm.org/D15676
>
>
>
> _______________________________________________
> 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/20151221/9c766dbb/attachment.html>


More information about the llvm-commits mailing list