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

[Devang Patel]

On Dec 16, 2009, at 12:13 AM, Victor Hernandez wrote:

> Nick,
>
> Thanks for the review.
>
> On Dec 15, 2009, at 10:47 PM, Nick Lewycky wrote:
>
>> Victor Hernandez wrote:
>>> Author: hernande
>>> Date: Tue Dec 15 20:52:09 2009
>>> New Revision: 91497
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=91497&view=rev
>>> Log:
>>> MDNodes that refer to an instruction are local to a function; in  
>>> that case, explicitly keep track of the function they are local to
>>
>> So what's the semantic here? What if the MDNode refers to an  
>> Instruction that gets spliced from one Function to another? What  
>> happens if the MDNode is attached to an Instruction that's RAUW'd  
>> with a Constant?
>>
>> Is this new field copied by MetadataContextImpl::copyMD?
>
> The semantic is that if an MDNode is created function-local, then it  
> will continue to be function-local even if its operands are modified  
> to no longer refer to any function-specific IR.

That'll break MDNode unique-ness.

Adding Function * in MDNode increases size of nodes and I anticipate  
that vast majority of nodes are not function local. A better approach  
is to let utility function isFunctionLocal() iterate  element and  
return respective Function *, if one or more elements are instructions.

The verifier can use this utility to verify the entire module, instead  
of verifying metadata per function.
-
Devang

>  I need to add that documentation to Metadata.h.
> If an instruction is copied from on Function to another, any  
> function-local metadata that refers to it will have to point to the  
> new instruction (and update its LocalFunction), or that operand can  
> be replaced with null.  The changes to the copying logic and the  
> necessary asserts and verification are still in progress.
>
>> You also need to assert that any Instructions passed in also belong  
>> to LocalFunction.
>
> Good call.  I will add that assert.  I will also be adding  
> Verification of function-local MDNodes that verifies the same.
>
>>
>>> Modified:
>>>     llvm/trunk/include/llvm/Metadata.h
>>>     llvm/trunk/lib/VMCore/Metadata.cpp
>>>
>>> Modified: llvm/trunk/include/llvm/Metadata.h
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Metadata.h?rev=91497&r1=91496&r2=91497&view=diff
>>>
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> ====================================================================
>>> --- llvm/trunk/include/llvm/Metadata.h (original)
>>> +++ llvm/trunk/include/llvm/Metadata.h Tue Dec 15 20:52:09 2009
>>> @@ -111,13 +111,16 @@
>>>
>>>    ElementVH *Node;
>>>    unsigned NodeSize;
>>> +  Function *LocalFunction;
>>>
>>>  protected:
>>> -  explicit MDNode(LLVMContext&C, Value *const *Vals, unsigned  
>>> NumVals);
>>> +  explicit MDNode(LLVMContext&C, Value *const *Vals, unsigned  
>>> NumVals,
>>> +                  Function *LocalFunction = NULL);
>>>  public:
>>>    // Constructors and destructors.
>>>    static MDNode *get(LLVMContext&Context,
>>> -                     Value *const *Vals, unsigned NumVals);
>>> +                     Value *const *Vals, unsigned NumVals,
>>> +                     Function *LocalFunction = NULL);
>>>
>>>    /// ~MDNode - Destroy MDNode.
>>>    ~MDNode();
>>> @@ -130,6 +133,9 @@
>>>
>>>    /// getNumElements - Return number of MDNode elements.
>>>    unsigned getNumElements() const { return NodeSize; }
>>> +
>>> +  /// isFunctionLocal - Return whether MDNode is local to a  
>>> function.
>>> +  bool isFunctionLocal() const { return LocalFunction; }
>>>
>>>    /// Profile - calculate a unique identifier for this MDNode to  
>>> collapse
>>>    /// duplicates
>>>
>>> Modified: llvm/trunk/lib/VMCore/Metadata.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/VMCore/Metadata.cpp?rev=91497&r1=91496&r2=91497&view=diff
>>>
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> ====================================================================
>>> --- llvm/trunk/lib/VMCore/Metadata.cpp (original)
>>> +++ llvm/trunk/lib/VMCore/Metadata.cpp Tue Dec 15 20:52:09 2009
>>> @@ -49,13 +49,15 @@
>>>  // 
>>> = 
>>> = 
>>> = 
>>> ----------------------------------------------------------------------= 
>>> ==//
>>>  // MDNode implementation.
>>>  //
>>> -MDNode::MDNode(LLVMContext&C, Value *const *Vals, unsigned NumVals)
>>> +MDNode::MDNode(LLVMContext&C, Value *const *Vals, unsigned NumVals,
>>> +               Function *LocalFunction)
>>>    : MetadataBase(Type::getMetadataTy(C), Value::MDNodeVal) {
>>>    NodeSize = NumVals;
>>>    Node = new ElementVH[NodeSize];
>>>    ElementVH *Ptr = Node;
>>>    for (unsigned i = 0; i != NumVals; ++i)
>>>      *Ptr++ = ElementVH(Vals[i], this);
>>> +  LocalFunction = LocalFunction;
>>
>> Perhaps you meant this->LocalFunction = LocalFunction?
>
> Good catch. I fixed this in r91524.
>
>>
>> Nick
>>
>>>  }
>>>
>>>  void MDNode::Profile(FoldingSetNodeID&ID) const {
>>> @@ -63,17 +65,20 @@
>>>      ID.AddPointer(getElement(i));
>>>  }
>>>
>>> -MDNode *MDNode::get(LLVMContext&Context, Value*const* Vals,  
>>> unsigned NumVals) {
>>> +MDNode *MDNode::get(LLVMContext&Context, Value*const* Vals,  
>>> unsigned NumVals,
>>> +                    Function *LocalFunction) {
>>>    LLVMContextImpl *pImpl = Context.pImpl;
>>>    FoldingSetNodeID ID;
>>>    for (unsigned i = 0; i != NumVals; ++i)
>>>      ID.AddPointer(Vals[i]);
>>> +  if (LocalFunction)
>>> +    ID.AddPointer(LocalFunction);
>>>
>>>    void *InsertPoint;
>>>    MDNode *N = pImpl->MDNodeSet.FindNodeOrInsertPos(ID,  
>>> InsertPoint);
>>>    if (!N) {
>>>      // InsertPoint will have been set by the FindNodeOrInsertPos  
>>> call.
>>> -    N = new MDNode(Context, Vals, NumVals);
>>> +    N = new MDNode(Context, Vals, NumVals, LocalFunction);
>>>      pImpl->MDNodeSet.InsertNode(N, InsertPoint);
>>>    }
>>>    return N;
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>
>>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20091216/f7983fd0/attachment.html>