<html><head><meta http-equiv="content-type" content="text/html; charset=utf-8"></head><body dir="auto"><div><br><br>Sent from my iPhone</div><div><br>On Jan 5, 2015, at 5:21 AM, Chandler Carruth <<a href="mailto:chandlerc@google.com">chandlerc@google.com</a>> wrote:<br><br></div><blockquote type="cite"><div><div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jan 5, 2015 at 3:09 AM, David Majnemer <span dir="ltr"><<a href="mailto:david.majnemer@gmail.com" target="_blank">david.majnemer@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><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 dir="auto"><div><br></div><div>The idea of improving the inliner is also great, but you may find that it's needed for cases other than this one if i'm right about the instcombine.</div></div></blockquote><div><br></div></span><div>Sadly, the combine fails because InstCombine queries CastInst::isBitCastable to determine the castable-ness of the parameter type and the argument type. It isn't bitcastable though, it's ptrtoint/inttoptr castable.</div><div><br></div><div>The following patch opens up the optimization:</div><div><div>--- a/lib/Transforms/InstCombine/InstCombineCalls.cpp</div><div>+++ b/lib/Transforms/InstCombine/InstCombineCalls.cpp</div><div>@@ -1456,7 +1456,7 @@ bool InstCombiner::transformConstExprCastCall(CallSite CS) {</div><div> Type *ParamTy = FT->getParamType(i);</div><div> Type *ActTy = (*AI)->getType();</div><div> </div><div>- if (!CastInst::isBitCastable(ActTy, ParamTy))</div><div>+ if (!CastInst::isBitOrNoopPointerCastable(ActTy, ParamTy, DL))</div><div> return false; // Cannot transform this parameter value.</div><div> </div><div> if (AttrBuilder(CallerPAL.getParamAttributes(i + 1), i + 1).</div><div>@@ -1551,7 +1551,7 @@ bool InstCombiner::transformConstExprCastCall(CallSite CS) {</div><div> if ((*AI)->getType() == ParamTy) {</div><div> Args.push_back(*AI);</div><div> } else {</div><div>- Args.push_back(Builder->CreateBitCast(*AI, ParamTy));</div><div>+ Args.push_back(Builder->CreateBitOrPointerCast(*AI, ParamTy));</div><div> }</div><div> </div><div> // Add any parameter attributes.</div></div></blockquote></div><br>This is exactly why I added those casting utilities. With sufficient testing, this patch LGTM as a way to effectively canonicalize these cases. Need to make sure this handles all the cases of ptr->int and int->ptr with too large, too small, and just right integer sizes. If all goes well, this seems like a really nice improvement.</div></div>
</div></blockquote>Nice! Can I request we add addrspacecast to the list too. It's probably in the utilities you mention but just needs a test case for this new use of the utility.<div><br></div><div>Pete</div></body></html>