[llvm] r262682 - Fix a memory leak.

[David Blaikie via llvm-commits]

On Fri, Mar 4, 2016 at 5:37 PM, Easwaran Raman <eraman at google.com> wrote:

> So I need to make the constructor of BlockFrequencyAnalysis out of line,
> not just the dtor since somehow BlockFrequencyAnalysis's default
> compiler-generated constructor instantiates DenseMap's destructor. I am
> quite flummoxed by this. In an attempt to understand this, I compiled this
> with gcc and looked at the earliest gimple dump which showed an empty try
> block with a catch block that calls the DenseMap's destructor.
>

Yep, don't think you can get around that, even if you = default the ctor
(though in theory it would be nice if the last non-trivial member in the
list didn't need to have its dtor accessible in a defaulted ctor (because
nothing could execute after it during construction, so there would never be
a need to clean it up))


>
> Anyway, I've sent out a patch.
>

Thanks!


>
> - Easwaran
>
>
> On Fri, Mar 4, 2016 at 3:29 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>> Depends where the code instantiate's unique_ptr's dtor - if unique_ptr's
>> dtor is only needed in InlineCost's dtor, then if you make the InlineCost
>> dtor out of line (In the .cpp file) you wouldn't need the extra #includes.
>> We do this in other places across LLVM.
>>
>> On Fri, Mar 4, 2016 at 3:27 PM, Easwaran Raman <eraman at google.com> wrote:
>>
>>> AFAICT, using unique_ptr would require including BlockFrequencyInfo.h
>>> and BlockFrequencyInfoImpl.h in InlineCost.h (instantiation of operator()
>>> in default_delete requires complete type). I thought that since the use
>>> of BlockFrequencyInfo* is fairly isolated within BlockFrequencyAnalysis, it
>>> is probably ok to not use the unique_ptr and avoid these two header file
>>> inclusions. Thoughts?
>>>
>>>
>>>
>>> On Fri, Mar 4, 2016 at 1:41 PM, David Blaikie <dblaikie at gmail.com>
>>> wrote:
>>>
>>>> Any chance of using unique_ptr instead?
>>>>
>>>> On Thu, Mar 3, 2016 at 5:18 PM, Easwaran Raman via llvm-commits <
>>>> llvm-commits at lists.llvm.org> wrote:
>>>>
>>>>> Author: eraman
>>>>> Date: Thu Mar  3 19:18:40 2016
>>>>> New Revision: 262682
>>>>>
>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=262682&view=rev
>>>>> Log:
>>>>> Fix a memory leak.
>>>>>
>>>>> Modified:
>>>>>     llvm/trunk/lib/Analysis/InlineCost.cpp
>>>>>
>>>>> Modified: llvm/trunk/lib/Analysis/InlineCost.cpp
>>>>> URL:
>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/InlineCost.cpp?rev=262682&r1=262681&r2=262682&view=diff
>>>>>
>>>>> ==============================================================================
>>>>> --- llvm/trunk/lib/Analysis/InlineCost.cpp (original)
>>>>> +++ llvm/trunk/lib/Analysis/InlineCost.cpp Thu Mar  3 19:18:40 2016
>>>>> @@ -1600,5 +1600,8 @@ BlockFrequencyInfo *BlockFrequencyAnalys
>>>>>
>>>>>  /// \brief Invalidate BlockFrequencyInfo for a function.
>>>>>  void BlockFrequencyAnalysis::invalidateBlockFrequencyInfo(Function
>>>>> *F) {
>>>>> -  BFM.erase(F);
>>>>> +  if (BFM.count(F)) {
>>>>> +    delete BFM[F];
>>>>> +    BFM.erase(F);
>>>>> +  }
>>>>>  }
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> llvm-commits mailing list
>>>>> llvm-commits at lists.llvm.org
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>>
>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160307/a36e918a/attachment.html>