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

Mehdi Amini via llvm-commits llvm-commits at lists.llvm.org
Sun Dec 20 20:42:20 PST 2015


> On Dec 20, 2015, at 4:06 PM, Philip Reames <listmail at philipreames.com> wrote:
> 
> 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.

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.

— 
Mehdi




> 
> 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 <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 <mailto:llvm-commits at lists.llvm.org>
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <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/a9d481b0/attachment.html>


More information about the llvm-commits mailing list