[llvm-commits] preserve pointer roots in globals

[Nick Lewycky]

On 30 June 2012 13:52, Nuno Lopes <nunoplopes at sapo.pt> wrote:

> Hi Nick,
>
> I understand the use case that you and Chandler presented in the other
> emails, and I believe it's a perfectly valid one!
> However, I've 2 concerns about this patch:
> - it seems there's some code duplication between CleanupPointerRootUsers()
> and CleanupConstantGlobalUsers(). Can't you just single out the couple of
> cases where the behavior is different?
>

Maybe. They're accomplishing different things and they test different
conditions along the way. I'll see what I can do.


> - I'm not totally confortable with disabling such class of optimizations.
> I believe the patch should learn a few tricks to remove malloc calls before
> going in. Otherwise I fear that no one will revisit this issue for a long
> time..  If the pointer doesn't escape anywhere else besides the store to
> the global, then it can be safely removed. That's hopefully easy to
> implement.
>

I've thought about this for a while and I have a new crazy idea. What if I
move all this dead-malloc work to a new malloc optimization pass. We need
one of these anyways. We should have a pass that turns unescaping small
mallocs into allocas, and if the pointer is captured by a global we can
check whether the global is never-loaded and decide to delete it after all.
Yes globalopt already knows that the global is never loaded, but that's
relatively easy to calculate and I don't want to have two passes (globalopt
and heap-stack-promotion) doing relatively expensive capture tracking.

Does this make sense? Or have I gone off the deep end?


> BTW, I would add a few more comments to the code explaining the reason for
> the behavior being introduced, to make sure no one accidently reverts the
> patch in a few years.
>

I'm hoping that using names like "isLeakCheckerRoot()" will make it clear
what's going on.

Nick


>
> Nuno
>
>
> -----Original Message----- From: Nick Lewycky
> Sent: Friday, June 29, 2012 4:08 AM
> To: Commit Messages and Patches for LLVM
> Subject: [llvm-commits] preserve pointer roots in globals
>
>
> The attached patch improves the interaction between GlobalOpt and leak
> checkers. One of the things that GlobalOpt will do is delete all the stores
> to a global that isn't loaded. If for some reason we don't mop up the
> malloc/new which was being stored in that global, a leak checker will
> complain.
>
> The patch handles this by not deleting stores to globals in the above
> case, when the global is known to be a pointer type, or a composite type
> that contains one. We do delete stores of Constant, since those are
> trivially not derived from calling an allocator. There's more analysis we
> could do here to win back optimization if it turns out to be really
> important.
>
> Please review!
>
> Nick
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120710/61cb08cf/attachment.html>