[PATCH] Address Space Casting

[David Chisnall]

================
Comment at: lib/IR/Verifier.cpp:1445-1446
@@ +1444,4 @@
+          "AddrSpaceCast result must be a pointer", &I);
+  Assert1(SrcTy->getPointerAddressSpace() != DestTy->getPointerAddressSpace(),
+          "AddrSpaceCast must be between different address spaces", &I);
+  visitInstruction(I);
----------------
Michele Scandale wrote:
> David Chisnall wrote:
> > Michele Scandale wrote:
> > > Matt Arsenault wrote:
> > > > I'm not sure this should be disallowed. In cases where the address spaces could differ, it could be easier for a frontend to just always emit addrspace cast, which could be turned into bitcast when possible. In that case it might be better to call this ptrcast, and bitcast could be used in the case when the address spaces match.
> > > From the specification that David summarized, 'addrspacecast' should be used only when address spaces are different, while bitcast should be used when the address space is the same.
> > > 
> > > I agree with you that generalizing the semantic would increase the flexibility... I'll fix the patch in this direction.
> > I think this is solved.  The front end will always use the IRBuilder method to emit a pointer cast and it will emit the correct cast instruction.  Let's keep the IR semantically rich and provide APIs to make it easy for front ends.
> So you're saying that it's not necessary to relax the semantic of addrspacecast instruction, right?
Yes, I think this is fine as-is.

================
Comment at: lib/IR/ConstantFold.cpp:692-693
@@ -691,2 +691,4 @@
     return FoldBitCast(V, DestTy);
+  case Instruction::AddrSpaceCast:
+    return 0;
   }
----------------
Michele Scandale wrote:
> David Chisnall wrote:
> > Michele Scandale wrote:
> > > Matt Arsenault wrote:
> > > > Are addrspace casts allowed between the same address space? If so, you could transform addrspace cast to the same address space into a bitcast
> > > With respect to the specification no: the addrspacecast is used only between differen address spaces
> > It would be nice to have the verifier enforce that invariant.
> Is not enough the modification on the Verifier?
Sorry, I forgot to delete this comment after getting to the verifier code.

================
Comment at: lib/IR/Instructions.cpp:2099
@@ -2098,2 +2098,3 @@
+    case Instruction::AddrSpaceCast: // TODO-addrspace: check this
       return false; // These always modify bits
     case Instruction::BitCast:
----------------
Michele Scandale wrote:
> David Chisnall wrote:
> > If we have the TargetLowering information here, we can return whether it's a no-op cast.
> To allow this we would need to add an hook in TargetTransformInfo and add an optional argument to the CastInst::isNoopCast method to pass a pointer to TargetTransformInfo object... but I'm not sure if it's a reasonable thing to be done here.
It's worth doing, but it's not worth doing now.  Just add a comment that it would be nice.

================
Comment at: include/llvm/Target/TargetLowering.h:826
@@ +825,3 @@
+  virtual bool isNoopAddrSpaceCast(unsigned SrcAS, unsigned DestAS) const {
+    return false;
+  }
----------------
Michele Scandale wrote:
> David Chisnall wrote:
> > It might be better to make this default to true and have targets that want to use this overload the method - currently, most archs we support are single address space.
> For targets with a single address space only address space zero should be seen by the backend, that's why I chose that value.
That makes sense.


http://llvm-reviews.chandlerc.com/D1401