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

[Victor Hernandez]

On Jan 15, 2010, at 2:56 PM, Chris Lattner wrote:

> 
> 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.

After discussing this further with Chris, I am going to leave isFunctionLocal() and getFunction() as is.  (Except for the getFunction() comment I just added in r93802.)  Devang's concern is that the creation of all MDNodes is slowed down by having to visit the operands to set the isFunctionLocal() bit properly.  The operand visit is not that expensive (only need to look at first-depth of operands since function-local metadata cannot point to itself), and the presence of the isFunctionLocal() bit makes isFunctionLocal() and getFunction() fast for non-function-local metadata, the common case.

> 
> -Chris
>