[llvm-commits] [PATCH] InstCombine: remove malloc+free if malloc's only uses are comparisons to null

[Matti Niemenmaa]

On 2010-05-17 12:12, Duncan Sands wrote:
>> There's one slight weirdness in it whose cleanness I'm not so sure
>> about: when getting rid of the malloc+free, we want to replace the
>> remaining uses of the malloc with a constant (trivially folded) non-null
>> pointer. The only way of getting such a thing I could think of was to
>> bitcast the enclosing function's address, since that's a GlobalValue and
>> hence can be handled by ConstantFoldCompareInstruction. It definitely
>> works, but it seems a bit hacky.
>
> why not just directly replace the comparisons with true/false?  Also,
> InstCombineCompares.cpp has a similar transformation in which a malloc
> which is never freed, but only used in comparisons (for some reason it
> is limited to only one comparison) is removed.  You may want to share
> some code/logic with that.

I did it like that mostly to make the code simpler, since it's then just 
a single RAUW to get rid of the malloc. I suppose you're right that it's 
cleaner to replace the icmps themselves, even though it takes some more 
code.

Another issue was that I wasn't sure what to do with comparisons that 
aren't eq/ne, since the value of null is target-specific. Apparently 
ConstantFoldCompareInstruction does nothing to them anyway. I think the 
safest thing to do is to bail on such comparisons and not perform the 
optimization. I'll amend the patch to only operate on icmp eq/ne.

The code in InstCombineCompares.cpp makes me wonder whether we should be 
operating on the malloc call to begin with? That is, move this 'malloc + 
[icmps] + [free]' removal from visitFree and InstCombineCompares.cpp to 
a new InstCombiner::visitMalloc function which would be called like 
visitFree currently is, from InstCombiner::visitCallInst.