[llvm-commits] [llvm] r91497 - in /llvm/trunk: include/llvm/Metadata.h lib/VMCore/Metadata.cpp
Devang Patel
dpatel at apple.com
Wed Dec 16 10:56:16 PST 2009
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>
More information about the llvm-commits
mailing list