<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><br><div><div>On Mar 5, 2013, at 9:58 PM, Chandler Carruth <<a href="mailto:chandlerc@google.com">chandlerc@google.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div class="gmail_extra" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;"><div class="gmail_quote"><br class="Apple-interchange-newline">On Tue, Mar 5, 2013 at 9:44 PM, Jim Grosbach<span class="Apple-converted-space"> </span><span dir="ltr"><<a href="mailto:grosbach@apple.com" target="_blank" class="cremed">grosbach@apple.com</a>></span><span class="Apple-converted-space"> </span>wrote:<br><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><div id=":ne">+ // If the allocation has multiple uses, only promote it if we're not<br>+ // shrinking the amount of memory being allocated.<br>+ uint64_t AllocElTyStoreSize = TD->getTypeStoreSize(AllocElTy);<br>+ uint64_t CastElTyStoreSize = TD->getTypeStoreSize(CastElTy);<br>+ if (!AI.hasOneUse() && CastElTyStoreSize < AllocElTyStoreSize) return 0;<br>+</div></blockquote></div><br>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?</div><div class="gmail_extra" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;"><br></div><div class="gmail_extra" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;">%a = alloca i32</div><div class="gmail_extra" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;">%a2 = bitcast i32* %a to i8*</div><div class="gmail_extra" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;">%a3 = bitcast i8* %a2 to i32*</div><div class="gmail_extra" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;">%boom = load i32* %a3</div></blockquote></div><br><div>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.</div><div><br></div><div>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.</div><div><br></div><div>What do you think?</div><div><br></div><div>-Jim</div></body></html>