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

Devang Patel dpatel at apple.com
Fri Mar 5 09:48:32 PST 2010


On Mar 4, 2010, at 6:11 PM, Jeffrey Yasskin wrote:

> On Thu, Feb 18, 2010 at 12:53 PM, Devang Patel <dpatel at apple.com> wrote:
>> Author: dpatel
>> Date: Thu Feb 18 14:53:16 2010
>> New Revision: 96609
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=96609&view=rev
>> Log:
>> Destroy MDNodes gracefully while deleting llvm context.
> 
> This change doesn't accomplish that. When one of an MDNode's operands
> turns to NULL, it's removed from the MDNodeSet,

I do not think so. Did I miss something ?

> which means the "while
> (!MDNodeSet.empty())" loop never executes.
> 
> I'm working on a real fix, and from reading the code, the
> replaceAllOperandsWithNull() step may not be necessary. Just calling
> MDNode::destroy() looks like it should work fine, even with operands
> still present and with cycles in the MDNode graph. What am I missing?

That'd be r95903, which was reverted due to llvmgcc bootstrap failure. Destroying a MDNode A triggers change in another MDNode B, A is one of the operands of B. This will kick in "enforce MDNode uniqueness" engine and so it'll run into a cycle where MDNode that is being destroyed is also being replaced.

-
Devang

> 
> Btw, I'm looking for memory leaks using valgrind-3.5.0 installed from
> macports. I found this one with `valgrind --dsymutil=yes
> --leak-check=full unittests/VMCore/Debug/VMCoreTests`
> 
>> Modified:
>>    llvm/trunk/include/llvm/Metadata.h
>>    llvm/trunk/lib/VMCore/LLVMContextImpl.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=96609&r1=96608&r2=96609&view=diff
>> 
>> ==============================================================================
>> --- llvm/trunk/include/llvm/Metadata.h (original)
>> +++ llvm/trunk/include/llvm/Metadata.h Thu Feb 18 14:53:16 2010
>> @@ -75,6 +75,7 @@
>>   MDNode(const MDNode &);                // DO NOT IMPLEMENT
>>   void operator=(const MDNode &);        // DO NOT IMPLEMENT
>>   friend class MDNodeOperand;
>> +  friend class LLVMContextImpl;
>> 
>>   /// NumOperands - This many 'MDNodeOperand' items are co-allocated onto the
>>   /// end of this MDNode.
>> @@ -106,6 +107,9 @@
>>   // Replace each instance of F from the operand list of this node with T.
>>   void replaceOperand(MDNodeOperand *Op, Value *NewVal);
>>   ~MDNode();
>> +    // replaceAllOperandsWithNull - This is used while destroying llvm context to
>> +  // gracefully delete all nodes. This method replaces all operands with null.
>> +  void replaceAllOperandsWithNull();
>> 
>>  protected:
>>   explicit MDNode(LLVMContext &C, Value *const *Vals, unsigned NumVals,
>> 
>> Modified: llvm/trunk/lib/VMCore/LLVMContextImpl.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/VMCore/LLVMContextImpl.h?rev=96609&r1=96608&r2=96609&view=diff
>> 
>> ==============================================================================
>> --- llvm/trunk/lib/VMCore/LLVMContextImpl.h (original)
>> +++ llvm/trunk/lib/VMCore/LLVMContextImpl.h Thu Feb 18 14:53:16 2010
>> @@ -229,13 +229,23 @@
>>       if (I->second->use_empty())
>>         delete I->second;
>>     }
>> -    MDNodeSet.clear();
>>     AlwaysOpaqueTy->dropRef();
>>     for (OpaqueTypesTy::iterator I = OpaqueTypes.begin(), E = OpaqueTypes.end();
>>         I != E; ++I) {
>>       (*I)->AbstractTypeUsers.clear();
>>       delete *I;
>>     }
>> +    // Destroy MDNode operands first.
>> +    for (FoldingSetIterator<MDNode> I = MDNodeSet.begin(), E = MDNodeSet.end();
>> +         I != E;) {
>> +      MDNode *N = &(*I);
>> +      ++I;
>> +      N->replaceAllOperandsWithNull();
>> +    }
>> +    while (!MDNodeSet.empty()) {
>> +      MDNode *N = &(*MDNodeSet.begin());
>> +      N->destroy();
>> +    }
>>   }
>>  };
>> 
>> 
>> Modified: llvm/trunk/lib/VMCore/Metadata.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/VMCore/Metadata.cpp?rev=96609&r1=96608&r2=96609&view=diff
>> 
>> ==============================================================================
>> --- llvm/trunk/lib/VMCore/Metadata.cpp (original)
>> +++ llvm/trunk/lib/VMCore/Metadata.cpp Thu Feb 18 14:53:16 2010
>> @@ -257,6 +257,13 @@
>>     ID.AddPointer(getOperand(i));
>>  }
>> 
>> +// replaceAllOperandsWithNull - This is used while destroying llvm context to
>> +// gracefully delete all nodes. This method replaces all operands with null.
>> +void MDNode::replaceAllOperandsWithNull() {
>> +  for (MDNodeOperand *Op = getOperandPtr(this, 0), *E = Op+NumOperands;
>> +       Op != E; ++Op)
>> +    replaceOperand(Op, 0);
>> +}
>> 
>>  // Replace value from this node's operand list.
>>  void MDNode::replaceOperand(MDNodeOperand *Op, Value *To) {
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> 





More information about the llvm-commits mailing list