[PATCH] Address Space Casting

[David Chisnall]

  This looks great.  We also need to add a clang diff that will ensure that it always emits addrspacecast instructions in the correct place.  I have some code in our branch that ensures that clang always emits inttoptr<->ptrtoints for these, so I can easily tidy it up and add it.


================
Comment at: include/llvm-c/Core.h:224
@@ -223,2 +223,3 @@
   LLVMBitCast        = 41,
+  LLVMAddrSpaceCast  = 42,
 
----------------
This changes the ABI for the C bindings.  Is this something we allow?

================
Comment at: include/llvm/Target/TargetLowering.h:826
@@ +825,3 @@
+  virtual bool isNoopAddrSpaceCast(unsigned SrcAS, unsigned DestAS) const {
+    return false;
+  }
----------------
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.

================
Comment at: lib/IR/ConstantFold.cpp:692-693
@@ -691,2 +691,4 @@
     return FoldBitCast(V, DestTy);
+  case Instruction::AddrSpaceCast:
+    return 0;
   }
----------------
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.

================
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:
----------------
If we have the TargetLowering information here, we can return whether it's a no-op cast.

================
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:
> 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.

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:1756
@@ +1755,3 @@
+  return SrcAS < 256 && DestAS < 256;
+}
+
----------------
It might be nice to have an assert here to check that DestAS != SrcAS.


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