[llvm-commits] [llvm] r99400 - in /llvm/trunk: lib/CodeGen/LiveIntervalAnalysis.cpp test/CodeGen/Generic/2010-03-24-liveintervalleak.ll

[Török Edwin]

On 03/30/2010 12:54 PM, Török Edwin wrote:
> On 03/26/2010 02:29 AM, Jakob Stoklund Olesen wrote:
>> On Mar 25, 2010, at 4:48 PM, Evan Cheng wrote:
>>
>>> On Mar 25, 2010, at 4:39 PM, Jakob Stoklund Olesen wrote:
>>>
>>>> On Mar 25, 2010, at 4:32 PM, Evan Cheng wrote:
>>>>
>>>>> On Mar 25, 2010, at 4:30 PM, Jakob Stoklund Olesen wrote:
>>>>>
>>>>>> On Mar 24, 2010, at 6:50 AM, Torok Edwin wrote:
>>>>>>
>>>>>>> Modified: llvm/trunk/lib/CodeGen/LiveIntervalAnalysis.cpp
>>>>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/LiveIntervalAnalysis.cpp?rev=99400&r1=99399&r2=99400&view=diff
>>>>>>> ==============================================================================
>>>>>>> --- llvm/trunk/lib/CodeGen/LiveIntervalAnalysis.cpp (original)
>>>>>>> +++ llvm/trunk/lib/CodeGen/LiveIntervalAnalysis.cpp Wed Mar 24 08:50:36 2010
>>>>>>> @@ -85,8 +85,10 @@
>>>>>>> void LiveIntervals::releaseMemory() {
>>>>>>> // Free the live intervals themselves.
>>>>>>> for (DenseMap<unsigned, LiveInterval*>::iterator I = r2iMap_.begin(),
>>>>>>> -       E = r2iMap_.end(); I != E; ++I)
>>>>>>> +       E = r2iMap_.end(); I != E; ++I) {
>>>>>>> +    I->second->clear();
>>>>>>> delete I->second;
>>>>>>> +  }
>>>>>>>
>>>>>>> r2iMap_.clear();
>>>>>> I think LiveInterval::clear is broken. I calls VNI->~VNInfo(), but those VNIs can be referenced from multiple intervals.
>>>>> Really? Multiple liveinterval's or liverange's?
>>>> LiveIntervals.
>>>>
>>>> Is a VNI supposed to belong to only one LiveInterval? I don't know any concrete examples where that is not the case, but a lot of funny stuff is going on when coalescing.
>>> I don't think VNI should be shared between liveinterval's. It may be shared temporarily during coalescing though.
>> Let's call it a bug. In that case, clear() is fine, but a VNI is accidentally shared somewhere.
>>
>>>  Either way, VNI are all bump allocated so the dtor should not be called?
>> Yeah, but they contain a SmallVector that leaks if it is not destroyed.
> 
> Adding more clear()s doesn't fix the leaks though, and sometimes I get
> double-frees.
> I don't really understand what happens with VNInfos after liveintervals
> are joined.
> 
> So instead of using clear() I tought BumpPtrAllocator to iterate over
> all the allocated slabs, and call a destructor function.
> In our case all the objects we allocate are of same size, so by simply
> iterating on the slabs with that size (and alignment) we find the
> pointers to all the VNInfos we allocated, and call their destructor.
> 
> The valnos list doesn't seem to be reliable for that purpose.
> 
> I'm testing the attached patch now.

I've committed it in r99883.
Let me know if it causes any problems (the initial version I committed
in 99881 broke the buildbots).

Best regards,
--Edwin