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

Philip Reames via llvm-commits llvm-commits at lists.llvm.org
Sun Dec 20 18:06:38 PST 2015


For some reason, I can't seem to log in to phabricator at the moment... 
A couple of comments below.

"InferFunctionAttrs" - bikeshed wise, this seems like a very generic, 
non-discriptive name.  Possibly: InferFunctionAttrsFromTLI?  Or 
something which otherwise gives a clue as to what we're infering from?

This really seems like it should somehow be a tablegen file.  I know 
you're just moving code around, but the long list of hand written code 
for each function smells.  Particularly the type checking of arguments 
after TLI already matched to an enum...

Stale comment in "inferAllPrototypeAttributes".

I'm uncomfortable with the addition of the various no-recurse 
attributes.  I'm not saying your choices are wrong, but mixing them in 
with a large code motion patch makes them hard to review. There's also a 
question of what we're assuming about the runtime environment.  After 
all, there is a perfectly legal (if unlikely) recursive implementation 
of strlen.

Otherwise, LGTM.

Philip



On 12/20/2015 03:13 AM, Chandler Carruth via llvm-commits wrote:
> chandlerc updated this revision to Diff 43324.
> chandlerc added a comment.
>
> Finish wiring up the new pass manager for inferring function attrs, and test it.
>
>
> http://reviews.llvm.org/D15676
>
> Files:
>    include/llvm/InitializePasses.h
>    include/llvm/Transforms/IPO/InferFunctionAttrs.h
>    lib/Passes/PassBuilder.cpp
>    lib/Passes/PassRegistry.def
>    lib/Transforms/IPO/CMakeLists.txt
>    lib/Transforms/IPO/FunctionAttrs.cpp
>    lib/Transforms/IPO/IPO.cpp
>    lib/Transforms/IPO/InferFunctionAttrs.cpp
>    lib/Transforms/IPO/PassManagerBuilder.cpp
>    test/Transforms/FunctionAttrs/2009-01-04-Annotate.ll
>    test/Transforms/FunctionAttrs/annotate-1.ll
>    test/Transforms/InferFunctionAttrs/annotate.ll
>    test/Transforms/InstCombine/strto-1.ll
>
>
>
> _______________________________________________
> 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/20151220/3d9b144b/attachment.html>


More information about the llvm-commits mailing list