<div dir="ltr">Sorry fro the commit necromancy, but when removing the code that was forming the bad bitcast here (and thus removing the utility of this test) I got really confused by the underlying IR....<br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Mar 13, 2014 at 3:51 PM, Owen Anderson <span dir="ltr"><<a href="mailto:resistor@mac.com" target="_blank">resistor@mac.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: resistor<br>
Date: Thu Mar 13 17:51:43 2014<br>
New Revision: 203862<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=203862&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=203862&view=rev</a><br>
Log:<br>
Fix a bug in InstCombine where we would incorrectly attempt to construct a<br>
bitcast between pointers of two different address spaces if they happened to have<br>
the same pointer size.<br></blockquote><div><br></div><div>Ok, while generally this makes sense -- we can't blindly bitcast between address spaces just because they have the same size -- the test case doesn't look like valid IR to begin with and the code forming the "bad" bit cast seems to be totally correct according to the spec of bitcasts. See below:</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Added: llvm/trunk/test/Transforms/InstCombine/load-addrspace-cast.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/load-addrspace-cast.ll?rev=203862&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/load-addrspace-cast.ll?rev=203862&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/InstCombine/load-addrspace-cast.ll (added)<br>
+++ llvm/trunk/test/Transforms/InstCombine/load-addrspace-cast.ll Thu Mar 13 17:51:43 2014<br>
@@ -0,0 +1,12 @@<br>
+; RUN: opt -instcombine -S < %s | FileCheck %s<br>
+target datalayout = "e-p:64:64:64-n8:16:32:64"<br>
+<br>
+define i32* @pointer_to_addrspace_pointer(i32 addrspace(1)** %x) nounwind {<br>
+; CHECK-LABEL: @pointer_to_addrspace_pointer(<br>
+; CHECK: load<br>
+; CHECK: addrspacecast<br>
+  %y = bitcast i32 addrspace(1)** %x to i32**<br>
+  %z = load i32** %y<br></blockquote><div><br></div><div>So, why isn't this garbage in, garbage out? This code is blindly loading the bits of an addrspace(1) pointer into an addrspace(0) pointer. That is *exactly* the semantics provided by bitcast! Transforming this into a bitcast seems completely "correct" in that it accurately represents the operation in the input IR, and the problem seems to be that the input IR is invalid.</div><div><br></div><div>Should code like this be rejected by the verifier? Or some other tool? Is that possible? Could we at least document that rinsing a pointer through memory from one address space to another is just as invalid as bitcasting?</div><div><br></div><div>My assumption is that this is just an overly reduced test case from a real bug? But maybe not? I'm somewhat worried that instcombine may have been papering over bugs somewhere else in LLVM by "fixing" a store -> load "bitcast" of across address spaces into a proper addrspace cast.... Please let me know if that is the case. I'm about to remove the code which was doing this entirely, and if that exposes some deep underlying bug I can add back in an addrspace_cast forming "fix" until the core issues is fixed.</div><div><br></div><div>-Chandler</div></div></div></div>