[LLVMdev] Leaks in PBQPBuilderWithCoalescing::build ?

[Lang Hames]

Oooh. Neat. Thanks Dave. Please go ahead and commit that.

Arnaud - I have no idea whether Dave's patch will help with this bug, but
it's certainly worth testing.

- Lang.

On Wed, Sep 10, 2014 at 4:10 PM, David Blaikie <dblaikie at gmail.com> wrote:

> While I'm not sure where the leak is, using some pre-canned memory
> management might help...
>
> Attached is a patch that changes this allocation to use shared_ptr,
> perhaps it'll address the bug?
>
> (ideally we shouldn't need the intrusive ref counting
> (std::enable_shared_from_this) but instead have a weak_set that has
> std::weak_ptr in it & implicitly removes elements as they become null
> (probably on a harvesting schedule, rather than with a direct callback as
> is currently implemented))​
>  pbqp_leak.diff
> <https://docs.google.com/file/d/0B0jpkch3iC_7TXFVU2hCcUpfZXM/edit?usp=drive_web>
> ​
>
> On Wed, Sep 10, 2014 at 3:19 PM, Arnaud A. de Grandmaison <
> arnaud.degrandmaison at arm.com> wrote:
>
>> Hi Lang,
>>
>>
>>
>> In PBQPBuilderWithCoalescing::build, around line 360, we have code
>> looking like:
>>
>>
>>
>> …
>>
>> PBQP::Vector newCosts(g.getNodeCosts(node));
>>
>> addPhysRegCoalesce(newCosts, pregOpt, cBenefit);
>>
>> g.setNodeCosts(node, newCosts);
>>
>> …
>>
>>
>>
>> I suspect the leak occurs around the setNodeCosts method, and I have
>> trouble understanding how it handles the case where the node already has
>> costs.
>>
>>
>>
>> It seems to me that:
>>
>> - we make of copy of the node’s costs (probably because someone else can
>> refer to it ?)
>>
>> - we modify the copy
>>
>> - we set the node’s new costs. But what is supposed to happen there,
>> especially in the CostAllocator part ?
>>
>>
>>
>> Could you shed some light there ?
>>
>>
>>
>> Thanks,
>>
>> Arnaud
>>
>>
>>
>>
>>
>>
>>
>> *From:* Lang Hames [mailto:lhames at gmail.com]
>> *Sent:* 10 September 2014 19:26
>> *To:* Arnaud De Grandmaison
>> *Subject:* Re: Leaks in PBQPBuilderWithCoalescing::build ?
>>
>>
>>
>> Thanks Arnaud!
>>
>>
>>
>> - Lang.
>>
>>
>>
>> On Wed, Sep 10, 2014 at 11:23 AM, Arnaud A. de Grandmaison <
>> arnaud.degrandmaison at arm.com> wrote:
>>
>> Hi Lang,
>>
>>
>>
>> For your information, the leak sanitizer found something when I committed
>> my patch:
>>
>>
>>
>>
>> http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap/builds/4506/steps/check-llvm%20asan/logs/stdio
>>
>>
>>
>> I will try to have a look at it.
>>
>>
>>
>> Cheers,
>>
>> --
>>
>> Arnaud A. de Grandmaison
>>
>>
>>
>>
>>
>> _______________________________________________
>> LLVM Developers mailing list
>> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140910/568877da/attachment.html>