[LLVMdev] Leaks in PBQPBuilderWithCoalescing::build ?

[Lang Hames]

Excellent. Thanks again Dave. And thanks for the testing Arnaud.

For the curious: I went back to look for the leak and it's obvious with
hindsight. PoolRef's destructor was deleting the PoolEntry when the
ref-count hit zero, but the assignment operator was not. This was an
oversight left over from when I moved responsibility for deleting
PoolEntries from PoolEntry itself out to PoolRef. It's all moot now though:
Dave's solution fixes this, and is much cleaner to boot.

- Lang.

On Thu, Sep 11, 2014 at 3:42 AM, Arnaud A. de Grandmaison <
arnaud.degrandmaison at arm.com> wrote:

> I confirm the shared_ptr change fixes the issue. I will recommit the
> aarch64/pbqp test then.
>
>
>
> Thanks David !
>
> --
>
> Arnaud
>
>
>
> *From:* David Blaikie [mailto:dblaikie at gmail.com]
> *Sent:* 11 September 2014 01:06
> *To:* Lang Hames
> *Cc:* Arnaud De Grandmaison; LLVM Developers Mailing List
> *Subject:* Re: [LLVMdev] Leaks in PBQPBuilderWithCoalescing::build ?
>
>
>
>
>
>
>
> On Wed, Sep 10, 2014 at 4:21 PM, Lang Hames <lhames at gmail.com> wrote:
>
> Oooh. Neat. Thanks Dave. Please go ahead and commit that.
>
>
>
> Committed in 217563.
>
> Running the PBQP test without my change under valgrind didn't show any
> memory leak, but hopefully with Arnaud's change and valgrind he can see the
> leak and then verify that my change addresses it... *fingers crossed*
>
>
>
>
>
> 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/20140911/ef84c2f6/attachment.html>