<div dir="ltr">On Wed, Sep 4, 2013 at 4:56 AM, David Chisnall <span dir="ltr"><<a href="mailto:David.Chisnall@cl.cam.ac.uk" target="_blank">David.Chisnall@cl.cam.ac.uk</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class=""><div class="h5">On 4 Sep 2013, at 00:01, Eli Friedman <<a href="mailto:eli.friedman@gmail.com">eli.friedman@gmail.com</a>> wrote:<br>

<br>
> On Tue, Sep 3, 2013 at 3:01 PM, Michele Scandale <<a href="mailto:michele.scandale@gmail.com">michele.scandale@gmail.com</a>> wrote:<br>
><br>
><br>
> ================<br>
> Comment at: lib/IR/AutoUpgrade.cpp:404<br>
> @@ +403,2 @@<br>
> +  return Opc;<br>
> +}<br>
> ----------------<br>
> Eli Friedman wrote:<br>
> > This isn't appropriate: this transformation changes the semantics of the code.  bitcast is explicitly not equivalent to addrspacecast; it's equivalent to a ptrtoint+inttoptr pair.<br>
> From what have been discussed before (<a href="http://lists.cs.uiuc.edu/pipermail/llvmdev/2013-August/064674.html" target="_blank">http://lists.cs.uiuc.edu/pipermail/llvmdev/2013-August/064674.html</a>) seems that before bitcast was used to convert two pointers between different address spaces... That's the reason for that code....<br>

><br>
> "bitcast" has always been defined in LangRef as a no-op.  The whole reason for adding addrspacecast is precisely because it isn't a no-op.  They are clearly not equivalent.<br>
<br>
</div></div>Bitcasts between address spaces are now undefined, so there are two choices for the upgrader:<br>
<br>
- Turn them into addrspacecasts<br>
- Leave the module invalid, in such a state that it will not pass the verifier<br>
<br>
It makes more sense to me to have the first behaviour, because the only case where the original bitcast would make sense is the case where the bitcast was used because there was no addrspacecast instruction.<br></blockquote>
<div><br></div><div>Or it could have been a ptrtoint+inttoptr pair originally, which would get turned into a bitcast by instcombine.<br><br>I don't see what's wrong with the choice of turning such a bitcast back into a ptrtoint+inttoptr pair.<br>
<br></div><div>-Eli<br></div></div></div></div>