[llvm-commits] [llvm] r93400 - in /llvm/trunk: include/llvm/Metadata.h lib/VMCore/Metadata.cpp

[Chris Lattner]

On Jan 14, 2010, at 12:15 PM, Victor Hernandez wrote:

>
> On Jan 14, 2010, at 10:20 AM, Victor Hernandez wrote:
>
>>
>> On Jan 14, 2010, at 9:39 AM, Devang Patel wrote:
>>
>>> On Wed, Jan 13, 2010 at 5:45 PM, Victor Hernandez <vhernandez at apple.com 
>>> > wrote:
>>>> Author: hernande
>>>> Date: Wed Jan 13 19:45:14 2010
>>>> New Revision: 93400
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=93400&view=rev
>>>> Log:
>>>> Add MDNode::getFunction(), which figures out the metadata's  
>>>> function, if it has function that it is local to.
>>>
>>> so will this replace isFunctionLocal() ?
>>
>> No, I don't think it should replace isFunctionLocal().   
>> getFunction() is an expensive operation that should not be used in  
>> performance-critical tasks (currently it is only used when printing  
>> out ll files).  isFunctionLocal() is fast and will very rarely give  
>> a false positive (when a metadata is created function-local, but  
>> then the operands are modified later).
>
> After our discussion, I am more amenable to replacing  
> isFunctionLocal() with getFunction().  I just committed a debug  
> version of getFunction() that asserts that function-local metadata  
> has only 1 parent function.

Can you elaborate on why?  Your description before (isfunctionlocal is  
trivial and fast, nothing important should call getFunction) makes  
perfect sense to me.  Please add a comment on getFunction() indicating  
that it is slow and should not be used for performance critical code.

-Chris