[llvm-commits] [llvm] r160602 - in /llvm/trunk: lib/Transforms/IPO/GlobalOpt.cpp test/Transforms/GlobalOpt/2009-11-16-BrokenPerformHeapAllocSRoA.ll test/Transforms/GlobalOpt/cleanup-pointer-root-users.ll

Nick Lewycky nicholas at mxc.ca
Tue Jul 24 11:36:29 PDT 2012


Duncan Sands wrote:
> Hi Nick,
>
> On 23/07/12 21:00, Nick Lewycky wrote:
>> Duncan Sands wrote:
>>> Hi Nick,
>>>
>>>> Teach globalopt to play nice with leak checkers. This is a
>>>> reapplication of
>>>> r160529 that was subsequently reverted. The fix was to not call
>>>> GV->eraseFromParent() right before the caller does the same. The
>>>> existing
>>>> testcases already caught this bug if run under valgrind.
>>>
>>> rather than scanning the variable looking for pointers in it, why not
>>> just
>>> remove stores as before, but not remove stores of values of pointer
>>> type?
>>
>> What if the value is a first-class aggregate structure containing a
>> pointer member?
>
> never happens in practice? I'm sure this is much less likely than the
> case of
> someone storing their pointer as an integer.
>
>> The other issue is that this way we'd need to look at all the stores
>> on the GV
>> instead of looking at the single GV. Now, technically GlobalOpt
>> already has to
>> do that anyway and I could add a new enum to StoredType to indicate
>> that it's
>> stored to once but not with a pointer, but I don't see what makes that an
>> improvement. Looking at the GV has the nice property that I can
>> explain to users
>> why their global wasn't being treated as a pointer root -- have you
>> tried making
>> your pointer root GV have pointer type?
>
> The explanation would be exactly the same...
>
>> If there's some benefit we can get from looking at each store, please
>> let me
>> know. :)
>
> Because you get to eliminate lots of stores, i.e. all those that don't have
> pointer type?

I don't get it, isn't that what we do now? If the global has pointer 
type then all the stores have to be pointer typed or else the IR is invalid.

Nick



More information about the llvm-commits mailing list