[llvm] r176547 - InstCombine: Don't shrink allocas when combining with a bitcast.

Jim Grosbach grosbach at apple.com
Tue Mar 5 22:10:39 PST 2013


On Mar 5, 2013, at 9:58 PM, Chandler Carruth <chandlerc at google.com> wrote:

> 
> On Tue, Mar 5, 2013 at 9:44 PM, Jim Grosbach <grosbach at apple.com> wrote:
> +  // If the allocation has multiple uses, only promote it if we're not
> +  // shrinking the amount of memory being allocated.
> +  uint64_t AllocElTyStoreSize = TD->getTypeStoreSize(AllocElTy);
> +  uint64_t CastElTyStoreSize = TD->getTypeStoreSize(CastElTy);
> +  if (!AI.hasOneUse() && CastElTyStoreSize < AllocElTyStoreSize) return 0;
> +
> 
> I may just be missing the point (i've not really thought about it deeply) but I saw this, and hand to ask: why is it safe to shrink the amount of memory being allocated even if the alloca only has one use?
> 
> %a = alloca i32
> %a2 = bitcast i32* %a to i8*
> %a3 = bitcast i8* %a2 to i32*
> %boom = load i32* %a3

It's basically relying on the rest of the combines being smart and merging chains like that into a single bitcast which then ends up at the final type on the alloca. Yes, that scares the ever-living crap out of me, too.

Conceptually speaking, I'm not opposed to a tighter restriction (i.e., get rid of the hasOneUse()). I just wanted to make as minimal a change as I could to start with. As the regression tests reminded me multiple times through working on this, there's a *lot* of underlying assumptions, tight coupling and ordering dependencies going on with these combines in order to get things to play nicely. I am worried about the potential for subtle performance regressions if we tighten this too much. test/Transforms/Inline/devirtualize.ll smacked me upside the head about that for a while.

What do you think?

-Jim
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130305/945d089f/attachment.html>


More information about the llvm-commits mailing list