[llvm-commits] preserve pointer roots in globals

[Nuno Lopes]

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?
- 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.

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.

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