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

[Victor Hernandez]

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

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