[PATCH] [InstCombine] Don't fold bitcast into store if it would need addrspacecast

[Richard Osborne]

  Tidy up based on review comments.

  Make use of getPointerAddressSpace(), separate out checks for the address space of the
  pointer and the address space of the pointer the pointer points to.

  OK to commit?

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

CHANGE SINCE LAST DIFF
  http://llvm-reviews.chandlerc.com/D3117?vs=7949&id=8054#toc

Files:
  lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
  test/Transforms/InstCombine/bitcast-store.ll

Index: lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
===================================================================
--- lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
+++ lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
@@ -508,36 +508,47 @@
   if (!SrcPTy->isIntegerTy() && !SrcPTy->isPointerTy())
     return 0;
 
-  // If the pointers point into different address spaces or if they point to
-  // values with different sizes, we can't do the transformation.
+  // If the pointers point into different address spaces don't do the
+  // transformation.
+  if (SrcTy->getAddressSpace() !=
+      cast<PointerType>(CI->getType())->getAddressSpace())
+    return 0;
+
+  // If the pointers point to values of different sizes don't do the
+  // transformation.
   if (!IC.getDataLayout() ||
-      SrcTy->getAddressSpace() !=
-        cast<PointerType>(CI->getType())->getAddressSpace() ||
       IC.getDataLayout()->getTypeSizeInBits(SrcPTy) !=
       IC.getDataLayout()->getTypeSizeInBits(DestPTy))
     return 0;
 
+  // If the pointers point to pointers to different address spaces don't do the
+  // transformation. It is not safe to use the addrspacecast instruction in this
+  // case since, depending on the target, addrspacecast may not be a no-op cast.
+  if (SrcPTy->isPointerTy() && DestPTy->isPointerTy() &&
+      SrcPTy->getPointerAddressSpace() != DestPTy->getPointerAddressSpace())
+    return 0;
+
   // Okay, we are casting from one integer or pointer type to another of
   // the same size.  Instead of casting the pointer before
   // the store, cast the value to be stored.
   Value *NewCast;
-  Value *SIOp0 = SI.getOperand(0);
   Instruction::CastOps opcode = Instruction::BitCast;
-  Type* CastSrcTy = SIOp0->getType();
+  Type* CastSrcTy = DestPTy;
   Type* CastDstTy = SrcPTy;
   if (CastDstTy->isPointerTy()) {
     if (CastSrcTy->isIntegerTy())
       opcode = Instruction::IntToPtr;
   } else if (CastDstTy->isIntegerTy()) {
-    if (SIOp0->getType()->isPointerTy())
+    if (CastSrcTy->isPointerTy())
       opcode = Instruction::PtrToInt;
   }
 
   // SIOp0 is a pointer to aggregate and this is a store to the first field,
   // emit a GEP to index into its first field.
   if (!NewGEPIndices.empty())
     CastOp = IC.Builder->CreateInBoundsGEP(CastOp, NewGEPIndices);
 
+  Value *SIOp0 = SI.getOperand(0);
   NewCast = IC.Builder->CreateCast(opcode, SIOp0, CastDstTy,
                                    SIOp0->getName()+".c");
   SI.setOperand(0, NewCast);
Index: test/Transforms/InstCombine/bitcast-store.ll
===================================================================
--- test/Transforms/InstCombine/bitcast-store.ll
+++ test/Transforms/InstCombine/bitcast-store.ll
@@ -3,19 +3,33 @@
 ; Instcombine should preserve metadata and alignment while
 ; folding a bitcast into a store.
 
-; CHECK: store i32 (...)** bitcast (i8** getelementptr inbounds ([5 x i8*]* @G, i64 0, i64 2) to i32 (...)**), i32 (...)*** %0, align 16, !tag !0
-
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64"
 
 %struct.A = type { i32 (...)** }
 
 @G = external constant [5 x i8*]
 
+; CHECK-LABEL: @foo
+; CHECK: store i32 (...)** bitcast (i8** getelementptr inbounds ([5 x i8*]* @G, i64 0, i64 2) to i32 (...)**), i32 (...)*** %0, align 16, !tag !0
 define void @foo(%struct.A* %a) nounwind {
 entry:
   %0 = bitcast %struct.A* %a to i8***
   store i8** getelementptr inbounds ([5 x i8*]* @G, i64 0, i64 2), i8*** %0, align 16, !tag !0
   ret void
 }
 
 !0 = metadata !{metadata !"hello"}
+
+; Check instcombine doesn't try and fold the following bitcast into the store.
+; This transformation would not be safe since we would need to use addrspacecast
+; and addrspacecast is not guaranteed to be a no-op cast.
+
+; CHECK-LABEL: @bar
+; CHECK: %cast = bitcast i8** %b to i8 addrspace(1)**
+; CHECK: store i8 addrspace(1)* %a, i8 addrspace(1)** %cast
+define void @bar(i8 addrspace(1)* %a, i8** %b) nounwind {
+entry:
+  %cast = bitcast i8** %b to i8 addrspace(1)**
+  store i8 addrspace(1)* %a, i8 addrspace(1)** %cast
+  ret void
+}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D3117.2.patch
Type: text/x-patch
Size: 4201 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140324/4ed3fb18/attachment.bin>