<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Dec 16, 2009, at 12:13 AM, Victor Hernandez wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div>Nick,</div><div><br></div><div>Thanks for the review.</div><div><br></div><div><div>On Dec 15, 2009, at 10:47 PM, Nick Lewycky wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div>Victor Hernandez wrote:<br><blockquote type="cite">Author: hernande<br></blockquote><blockquote type="cite">Date: Tue Dec 15 20:52:09 2009<br></blockquote><blockquote type="cite">New Revision: 91497<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">URL: <a href="http://llvm.org/viewvc/llvm-project?rev=91497&view=rev">http://llvm.org/viewvc/llvm-project?rev=91497&view=rev</a><br></blockquote><blockquote type="cite">Log:<br></blockquote><blockquote type="cite">MDNodes that refer to an instruction are local to a function; in that case, explicitly keep track of the function they are local to<br></blockquote><br>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?<br><br>Is this new field copied by MetadataContextImpl::copyMD?<font class="Apple-style-span"><font class="Apple-style-span" color="#144FAE"><br></font></font></div></blockquote><div><div><br></div><div>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. </div></div></div></div></blockquote><div><br></div><div>That'll break MDNode unique-ness.</div><div><br></div><div>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.</div><div><br></div><div>The verifier can use this utility to verify the entire module, instead of verifying metadata per function.</div><div>-</div><div>Devang</div><div><br></div><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><div><div> I need to add that documentation to Metadata.h.</div><div>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.</div><div><br></div></div></div><div><blockquote type="cite"><div>You also need to assert that any Instructions passed in also belong to LocalFunction.<br></div></blockquote><div><div><br></div><div>Good call. I will add that assert. I will also be adding Verification of function-local MDNodes that verifies the same.</div><div><br></div></div></div><div><blockquote type="cite"><div><br><blockquote type="cite">Modified:<br></blockquote><blockquote type="cite"> llvm/trunk/include/llvm/Metadata.h<br></blockquote><blockquote type="cite"> llvm/trunk/lib/VMCore/Metadata.cpp<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">Modified: llvm/trunk/include/llvm/Metadata.h<br></blockquote><blockquote type="cite">URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Metadata.h?rev=91497&r1=91496&r2=91497&view=diff">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Metadata.h?rev=91497&r1=91496&r2=91497&view=diff</a><br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">==============================================================================<br></blockquote><blockquote type="cite">--- llvm/trunk/include/llvm/Metadata.h (original)<br></blockquote><blockquote type="cite">+++ llvm/trunk/include/llvm/Metadata.h Tue Dec 15 20:52:09 2009<br></blockquote><blockquote type="cite">@@ -111,13 +111,16 @@<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite"> ElementVH *Node;<br></blockquote><blockquote type="cite"> unsigned NodeSize;<br></blockquote><blockquote type="cite">+ Function *LocalFunction;<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite"> protected:<br></blockquote><blockquote type="cite">- explicit MDNode(LLVMContext&C, Value *const *Vals, unsigned NumVals);<br></blockquote><blockquote type="cite">+ explicit MDNode(LLVMContext&C, Value *const *Vals, unsigned NumVals,<br></blockquote><blockquote type="cite">+ Function *LocalFunction = NULL);<br></blockquote><blockquote type="cite"> public:<br></blockquote><blockquote type="cite"> // Constructors and destructors.<br></blockquote><blockquote type="cite"> static MDNode *get(LLVMContext&Context,<br></blockquote><blockquote type="cite">- Value *const *Vals, unsigned NumVals);<br></blockquote><blockquote type="cite">+ Value *const *Vals, unsigned NumVals,<br></blockquote><blockquote type="cite">+ Function *LocalFunction = NULL);<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite"> /// ~MDNode - Destroy MDNode.<br></blockquote><blockquote type="cite"> ~MDNode();<br></blockquote><blockquote type="cite">@@ -130,6 +133,9 @@<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite"> /// getNumElements - Return number of MDNode elements.<br></blockquote><blockquote type="cite"> unsigned getNumElements() const { return NodeSize; }<br></blockquote><blockquote type="cite">+<br></blockquote><blockquote type="cite">+ /// isFunctionLocal - Return whether MDNode is local to a function.<br></blockquote><blockquote type="cite">+ bool isFunctionLocal() const { return LocalFunction; }<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite"> /// Profile - calculate a unique identifier for this MDNode to collapse<br></blockquote><blockquote type="cite"> /// duplicates<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">Modified: llvm/trunk/lib/VMCore/Metadata.cpp<br></blockquote><blockquote type="cite">URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/VMCore/Metadata.cpp?rev=91497&r1=91496&r2=91497&view=diff">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/VMCore/Metadata.cpp?rev=91497&r1=91496&r2=91497&view=diff</a><br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">==============================================================================<br></blockquote><blockquote type="cite">--- llvm/trunk/lib/VMCore/Metadata.cpp (original)<br></blockquote><blockquote type="cite">+++ llvm/trunk/lib/VMCore/Metadata.cpp Tue Dec 15 20:52:09 2009<br></blockquote><blockquote type="cite">@@ -49,13 +49,15 @@<br></blockquote><blockquote type="cite"> //===----------------------------------------------------------------------===//<br></blockquote><blockquote type="cite"> // MDNode implementation.<br></blockquote><blockquote type="cite"> //<br></blockquote><blockquote type="cite">-MDNode::MDNode(LLVMContext&C, Value *const *Vals, unsigned NumVals)<br></blockquote><blockquote type="cite">+MDNode::MDNode(LLVMContext&C, Value *const *Vals, unsigned NumVals,<br></blockquote><blockquote type="cite">+ Function *LocalFunction)<br></blockquote><blockquote type="cite"> : MetadataBase(Type::getMetadataTy(C), Value::MDNodeVal) {<br></blockquote><blockquote type="cite"> NodeSize = NumVals;<br></blockquote><blockquote type="cite"> Node = new ElementVH[NodeSize];<br></blockquote><blockquote type="cite"> ElementVH *Ptr = Node;<br></blockquote><blockquote type="cite"> for (unsigned i = 0; i != NumVals; ++i)<br></blockquote><blockquote type="cite"> *Ptr++ = ElementVH(Vals[i], this);<br></blockquote><blockquote type="cite">+ LocalFunction = LocalFunction;<br></blockquote><br>Perhaps you meant this->LocalFunction = LocalFunction?<br></div></blockquote><div><br></div>Good catch. I fixed this in r91524.</div><div><br><blockquote type="cite"><div><br>Nick<br><br><blockquote type="cite"> }<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite"> void MDNode::Profile(FoldingSetNodeID&ID) const {<br></blockquote><blockquote type="cite">@@ -63,17 +65,20 @@<br></blockquote><blockquote type="cite"> ID.AddPointer(getElement(i));<br></blockquote><blockquote type="cite"> }<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">-MDNode *MDNode::get(LLVMContext&Context, Value*const* Vals, unsigned NumVals) {<br></blockquote><blockquote type="cite">+MDNode *MDNode::get(LLVMContext&Context, Value*const* Vals, unsigned NumVals,<br></blockquote><blockquote type="cite">+ Function *LocalFunction) {<br></blockquote><blockquote type="cite"> LLVMContextImpl *pImpl = Context.pImpl;<br></blockquote><blockquote type="cite"> FoldingSetNodeID ID;<br></blockquote><blockquote type="cite"> for (unsigned i = 0; i != NumVals; ++i)<br></blockquote><blockquote type="cite"> ID.AddPointer(Vals[i]);<br></blockquote><blockquote type="cite">+ if (LocalFunction)<br></blockquote><blockquote type="cite">+ ID.AddPointer(LocalFunction);<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite"> void *InsertPoint;<br></blockquote><blockquote type="cite"> MDNode *N = pImpl->MDNodeSet.FindNodeOrInsertPos(ID, InsertPoint);<br></blockquote><blockquote type="cite"> if (!N) {<br></blockquote><blockquote type="cite"> // InsertPoint will have been set by the FindNodeOrInsertPos call.<br></blockquote><blockquote type="cite">- N = new MDNode(Context, Vals, NumVals);<br></blockquote><blockquote type="cite">+ N = new MDNode(Context, Vals, NumVals, LocalFunction);<br></blockquote><blockquote type="cite"> pImpl->MDNodeSet.InsertNode(N, InsertPoint);<br></blockquote><blockquote type="cite"> }<br></blockquote><blockquote type="cite"> return N;<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">_______________________________________________<br></blockquote><blockquote type="cite">llvm-commits mailing list<br></blockquote><blockquote type="cite"><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br></blockquote><blockquote type="cite"><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br></blockquote><blockquote type="cite"><br></blockquote><br></div></blockquote></div><br></div>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits<br></blockquote></div><br></body></html>