Teach SROA about addrspacecast.

[chandlerc]

The changes to PtrUseVisitor make sense (it's a generic tool) but I'm curious why the correct fix isn't to have instcombine nuke all addrspacecasts of allocas? They don't really make any sense to me...

[arsenm]

addrspacecasts of allocas are an important case in OpenCL 2.0. Your private allocations are from allocas in address space 0, which are then often casted to the non-zero generic address space for convenience of use. Accessing private allocations and access through a flat pointer is expensive, so we really want to be able to eliminate the cast to generic and alloca if possible. Right now SROA doesn't eliminate any of these common allocas because of the addrspacecast

[chandlerc]

Pretty sure all the addrspace stuff got fixed, but let me know if not.

[jingyue]

This patch is not in SROA yet. Do you mind I check it in?

I was bitten by the same issue in the NVPTX backend. Code patterns such as

%0 = alloca i32
%1 = addrspacecast i32* %0 to addrspace(4) i32* ; cast from generic to local so that later accesses can be much faster
... use %1 ...

will appear quite often after NVPTXFavorNonGenericAddrSpaces (http://llvm.org/docs/doxygen/html/NVPTXTargetMachine_8cpp_source.html#l00169) with some WIP checked in. It would be great if SROA can nuke these allocas across addrspacecasts.

[jingyue]

+chandlerc

[jingyue]

+Xuetian

[wengxt]

So this is a case that we currently want to handle in nvptx, which is not covered instcombine/sroa right now.

; ModuleID = '<stdin>'
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-v16:16:16-v32:32:32-v64:64:64-v128:128:128-n16:32:64"
target triple = "nvptx64-unknown-unknown"

%struct.S = type { i32, i32, i32 }

; Function Attrs: nounwind
define void @_Z11TakesStruct1SPi(%struct.S* byval nocapture readonly %input, i32* nocapture %output) #0 {
entry:
  %input1 = alloca %struct.S, align 8
  %0 = addrspacecast %struct.S* %input1 to %struct.S addrspace(5)*
  %input2 = addrspacecast %struct.S* %input to %struct.S addrspace(101)*
  %input3 = load %struct.S, %struct.S addrspace(101)* %input2, align 4
  store %struct.S %input3, %struct.S addrspace(5)* %0, align 8
  %1 = getelementptr inbounds %struct.S, %struct.S addrspace(5)* %0, i64 0, i32 1
  %2 = load i32, i32 addrspace(5)* %1, align 4
  store i32 %2, i32* %output, align 4
  ret void
}

include/llvm/Analysis/PtrUseVisitor.h

void visitStoreInst(StoreInst &SI) {
     if (SI.getValueOperand() == U->get())
       PI.setEscaped(&SI);
   }
 
   void visitBitCastInst(BitCastInst &BC) {
     enqueueUsers(BC);
   }
 
+  void visitAddrSpaceCastInst(AddrSpaceCastInst &ASC) {
+    enqueueUsers(ASC);
+  }
+
   void visitPtrToIntInst(PtrToIntInst &I) {
     PI.setEscaped(&I);
   }
 
   void visitGetElementPtrInst(GetElementPtrInst &GEPI) {
     if (GEPI.use_empty())
       return;
 

lib/Transforms/Scalar/SROA.cpp

private:
 
   void visitBitCastInst(BitCastInst &BC) {
     if (BC.use_empty())
       return markAsDead(BC);
 
     return Base::visitBitCastInst(BC);
   }
 
+  void visitAddrSpaceCastInst(AddrSpaceCastInst &ASC) {
+    if (ASC.use_empty())
+      return markAsDead(ASC);
+
+    return Base::visitAddrSpaceCastInst(ASC);
+  }
+
   void visitGetElementPtrInst(GetElementPtrInst &GEPI) {
     if (GEPI.use_empty())
       return markAsDead(GEPI);
 
     if (SROAStrictInbounds && GEPI.isInBounds()) {
       // FIXME: This is a manually un-factored variant of the basic code inside
       // of GEPs with checking of the inbounds invariant specified in the
       // langref in a very strict sense. If we ever want to enable
do {
         Size = std::max(Size, DL.getTypeStoreSize(Op->getType()));
         continue;
       }
 
       if (GetElementPtrInst *GEP = dyn_cast<GetElementPtrInst>(I)) {
         if (!GEP->hasAllZeroIndices())
           return GEP;
       } else if (!isa<BitCastInst>(I) && !isa<PHINode>(I) &&
-                 !isa<SelectInst>(I)) {
+                 !isa<SelectInst>(I) && !isa<AddrSpaceCastInst>(I)) {
         return I;
       }
 
       for (User *U : I->users())
         if (Visited.insert(cast<Instruction>(U)))
           Uses.push_back(std::make_pair(I, cast<Instruction>(U)));
     } while (!Uses.empty());
 
bool isInstInList(Instruction *I,
     do {
       if (Ptr == &AI)
         return true;
 
       if (BitCastInst *BCI = dyn_cast<BitCastInst>(Ptr))
         Ptr = BCI->getOperand(0);
       else if (GetElementPtrInst *GEPI = dyn_cast<GetElementPtrInst>(Ptr))
         Ptr = GEPI->getPointerOperand();
+      else if (AddrSpaceCastInst *ASC = dyn_cast<AddrSpaceCastInst>(Ptr))
+        Ptr = ASC->getOperand(0);
       else
         return false;
 
     } while (Visited.insert(Ptr));
 
     return false;
   }
 
do {
 
     // Stash this pointer if we've found an i8*.
     if (Ptr->getType()->isIntegerTy(8)) {
       Int8Ptr = Ptr;
       Int8PtrOffset = Offset;
     }
 
     // Peel off a layer of the pointer and update the offset appropriately.
-    if (Operator::getOpcode(Ptr) == Instruction::BitCast) {
+    unsigned Opc = Operator::getOpcode(Ptr);
+    if (Opc == Instruction::BitCast || Opc == Instruction::AddrSpaceCast) {
       Ptr = cast<Operator>(Ptr)->getOperand(0);
     } else if (GlobalAlias *GA = dyn_cast<GlobalAlias>(Ptr)) {
       if (GA->mayBeOverridden())
         break;
       Ptr = GA->getAliasee();
     } else {
       break;
     }
if (!OffsetPtr) {
 
     OffsetPtr = Int8PtrOffset == 0 ? Int8Ptr :
       IRB.CreateInBoundsGEP(Int8Ptr, IRB.getInt(Int8PtrOffset),
                             NamePrefix + "sroa_raw_idx");
   }
   Ptr = OffsetPtr;
 
   // On the off chance we were targeting i8*, guard the bitcast here.
-  if (Ptr->getType() != PointerTy)
-    Ptr = IRB.CreateBitCast(Ptr, PointerTy, NamePrefix + "sroa_cast");
+  if (Ptr->getType() != PointerTy) {
+    Ptr = IRB.CreatePointerBitCastOrAddrSpaceCast(Ptr, PointerTy,
+                                                  NamePrefix + "sroa_cast");
+  }
 
   return Ptr;
 }
 
 /// \brief Test whether we can convert a value from the old to the new type.
 ///
 /// This predicate should be used to guard calls to convertValue in order to
 /// ensure that we only try to convert viable values. The strategy is that we
bool visitStoreInst(StoreInst &SI) {
     return true;
   }
 
   bool visitBitCastInst(BitCastInst &BC) {
     enqueueUsers(BC);
     return false;
   }
 
+  bool visitAddrSpaceCastInst(AddrSpaceCastInst &ASC) {
+    enqueueUsers(ASC);
+    return false;
+  }
+
   bool visitGetElementPtrInst(GetElementPtrInst &GEPI) {
     enqueueUsers(GEPI);
     return false;
   }
 
   bool visitPHINode(PHINode &PN) {
     enqueueUsers(PN);
     return false;

test/Transforms/SROA/basictest.ll

entry:
   store i64 %X, i64* %B
   br label %L2
 
 L2:
   %Z = load i64* %B
   ret i64 %Z
 }
 
+define i64 @test2_addrspacecast(i64 %X) {
+; CHECK-LABEL: @test2_addrspacecast(
+; CHECK-NOT: alloca
+; CHECK: ret i64 %X
+
+entry:
+  %A = alloca [8 x i8]
+  %B = addrspacecast [8 x i8]* %A to i64 addrspace(1)*
+  store i64 %X, i64 addrspace(1)* %B
+  br label %L2
+
+L2:
+  %Z = load i64 addrspace(1)* %B
+  ret i64 %Z
+}
+
 define void @test3(i8* %dst, i8* %src) {
 ; CHECK-LABEL: @test3(
 
 entry:
   %a = alloca [300 x i8]
 ; CHECK-NOT:  alloca
 ; CHECK:      %[[test3_a1:.*]] = alloca [42 x i8]
 ; CHECK-NEXT: %[[test3_a2:.*]] = alloca [99 x i8]
entry:
   %cast1 = bitcast %opaque* %x to i8*
   %cast2 = bitcast { i64, i8* }* %a to i8*
   call void @llvm.memcpy.p0i8.p0i8.i32(i8* %cast2, i8* %cast1, i32 16, i32 1, i1 false)
   %gep = getelementptr inbounds { i64, i8* }* %a, i32 0, i32 0
   %val = load i64* %gep
   ret i32 undef
 }
 
+declare void @llvm.memcpy.p0i8.p1i8.i32(i8* nocapture, i8 addrspace(1)* nocapture, i32, i32, i1) nounwind
+
+define i32 @test19_addrspacecast(%opaque* %x) {
+; This input will cause us to try to compute a natural GEP when rewriting
+; pointers in such a way that we try to GEP through the opaque type. Previously,
+; a check for an unsized type was missing and this crashed. Ensure it behaves
+; reasonably now.
+; CHECK-LABEL: @test19_addrspacecast(
+; CHECK-NOT: alloca
+; CHECK: ret i32 undef
+
+entry:
+  %a = alloca { i64, i8* }
+  %cast1 = addrspacecast %opaque* %x to i8 addrspace(1)*
+  %cast2 = bitcast { i64, i8* }* %a to i8*
+  call void @llvm.memcpy.p0i8.p1i8.i32(i8* %cast2, i8 addrspace(1)* %cast1, i32 16, i32 1, i1 false)
+  %gep = getelementptr inbounds { i64, i8* }* %a, i32 0, i32 0
+  %val = load i64* %gep
+  ret i32 undef
+}
+
 define i32 @test20() {
 ; Ensure we can track negative offsets (before the beginning of the alloca) and
 ; negative relative offsets from offsets starting past the end of the alloca.
 ; CHECK-LABEL: @test20(
 ; CHECK-NOT: alloca
 ; CHECK: %[[sum1:.*]] = add i32 1, 2
 ; CHECK: %[[sum2:.*]] = add i32 %[[sum1]], 3
 ; CHECK: ret i32 %[[sum2]]