[PATCH] D115895: InstCombine: Gracefully handle more allocas in the wrong address space

[Matt Arsenault via Phabricator via llvm-commits]

arsenm created this revision.
arsenm added reviewers: jdoerfert, lebedev.ri, ronlieb.
Herald added subscribers: dexonsmith, hiraditya.
arsenm requested review of this revision.
Herald added a subscriber: wdng.
Herald added a project: LLVM.

Officially this is currently required to always use the datalayout's
alloca address space. This may change in the future, and it's cleaner
to propagate the existing alloca's addrspace anyway.

      

This is a triple fix. Initially the change in simplifyAllocaArraySize
would drop the address space, but produce output. Fixing this hit an
assertion in the cast combine.

      

This patch also makes the changes to handle this situation from
a33e12801279a947c74fdee2655b24480941fb39 <https://reviews.llvm.org/rGa33e12801279a947c74fdee2655b24480941fb39> dead, so eliminate
it. InstCombine should not take it upon itself to introduce
addrspacecasts, and preserve the original address space instead.


https://reviews.llvm.org/D115895

Files:
  llvm/include/llvm/IR/Instructions.h
  llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
  llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
  llvm/test/Transforms/InstCombine/alloca-in-non-alloca-as.ll


Index: llvm/test/Transforms/InstCombine/alloca-in-non-alloca-as.ll
===================================================================
--- llvm/test/Transforms/InstCombine/alloca-in-non-alloca-as.ll
+++ llvm/test/Transforms/InstCombine/alloca-in-non-alloca-as.ll
@@ -11,11 +11,9 @@
 define weak amdgpu_kernel void @__omp_offloading_802_ea0109_main_l8(i32* %a) {
 ; CHECK-LABEL: @__omp_offloading_802_ea0109_main_l8(
 ; CHECK-NEXT:  .master:
-; CHECK-NEXT:    [[TMP0:%.*]] = alloca i32*, align 1, addrspace(5)
-; CHECK-NEXT:    [[DOTSUB:%.*]] = bitcast i32* addrspace(5)* [[TMP0]] to i8 addrspace(5)*
-; CHECK-NEXT:    [[TMP1:%.*]] = addrspacecast i8 addrspace(5)* [[DOTSUB]] to i8*
-; CHECK-NEXT:    [[A_ON_STACK:%.*]] = addrspacecast i32* addrspace(5)* [[TMP0]] to i32**
-; CHECK-NEXT:    call void @use(i8* [[TMP1]], i32** [[A_ON_STACK]])
+; CHECK-NEXT:    [[TMP0:%.*]] = alloca i32*, align 1
+; CHECK-NEXT:    [[DOTSUB:%.*]] = bitcast i32** [[TMP0]] to i8*
+; CHECK-NEXT:    call void @use(i8* [[DOTSUB]], i32** [[TMP0]])
 ; CHECK-NEXT:    ret void
 ;
 .master:
@@ -31,10 +29,8 @@
 define void @spam(i64* %arg1) {
 ; CHECK-LABEL: @spam(
 ; CHECK-NEXT:  bb:
-; CHECK-NEXT:    [[ALLOCA1:%.*]] = alloca [0 x [30 x %struct.widget]], align 16, addrspace(5)
-; CHECK-NEXT:    [[ALLOCA1_SUB:%.*]] = getelementptr inbounds [0 x [30 x %struct.widget]], [0 x [30 x %struct.widget]] addrspace(5)* [[ALLOCA1]], i32 0, i32 0
-; CHECK-NEXT:    [[TMP0:%.*]] = addrspacecast [30 x %struct.widget] addrspace(5)* [[ALLOCA1_SUB]] to [30 x %struct.widget]*
-; CHECK-NEXT:    [[GEP:%.*]] = getelementptr inbounds [30 x %struct.widget], [30 x %struct.widget]* [[TMP0]], i64 0, i64 0
+; CHECK-NEXT:    [[ALLOCA1:%.*]] = alloca [0 x [30 x %struct.widget]], align 16
+; CHECK-NEXT:    [[GEP:%.*]] = getelementptr inbounds [0 x [30 x %struct.widget]], [0 x [30 x %struct.widget]]* [[ALLOCA1]], i64 0, i64 0, i64 0
 ; CHECK-NEXT:    call void @zot(%struct.widget* [[GEP]])
 ; CHECK-NEXT:    ret void
 ;
Index: llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
===================================================================
--- llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
+++ llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
@@ -183,7 +183,8 @@
   if (const ConstantInt *C = dyn_cast<ConstantInt>(AI.getArraySize())) {
     if (C->getValue().getActiveBits() <= 64) {
       Type *NewTy = ArrayType::get(AI.getAllocatedType(), C->getZExtValue());
-      AllocaInst *New = IC.Builder.CreateAlloca(NewTy, nullptr, AI.getName());
+      AllocaInst *New = IC.Builder.CreateAlloca(NewTy, AI.getAddressSpace(),
+                                                nullptr, AI.getName());
       New->setAlignment(AI.getAlign());
 
       // Scan to the end of the allocation instructions, to skip over a block of
@@ -199,21 +200,13 @@
       Type *IdxTy = IC.getDataLayout().getIntPtrType(AI.getType());
       Value *NullIdx = Constant::getNullValue(IdxTy);
       Value *Idx[2] = {NullIdx, NullIdx};
-      Instruction *NewI = GetElementPtrInst::CreateInBounds(
+      Instruction *GEP = GetElementPtrInst::CreateInBounds(
           NewTy, New, Idx, New->getName() + ".sub");
-      IC.InsertNewInstBefore(NewI, *It);
-
-      // Gracefully handle allocas in other address spaces.
-      if (AI.getType()->getPointerAddressSpace() !=
-          NewI->getType()->getPointerAddressSpace()) {
-        NewI =
-            CastInst::CreatePointerBitCastOrAddrSpaceCast(NewI, AI.getType());
-        IC.InsertNewInstBefore(NewI, *It);
-      }
+      IC.InsertNewInstBefore(GEP, *It);
 
       // Now make everything use the getelementptr instead of the original
       // allocation.
-      return IC.replaceInstUsesWith(AI, NewI);
+      return IC.replaceInstUsesWith(AI, GEP);
     }
   }
 
Index: llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
===================================================================
--- llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
+++ llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
@@ -157,7 +157,7 @@
     Amt = Builder.CreateAdd(Amt, Off);
   }
 
-  AllocaInst *New = Builder.CreateAlloca(CastElTy, Amt);
+  AllocaInst *New = Builder.CreateAlloca(CastElTy, AI.getAddressSpace(), Amt);
   New->setAlignment(AI.getAlign());
   New->takeName(&AI);
   New->setUsedWithInAlloca(AI.isUsedWithInAlloca());
Index: llvm/include/llvm/IR/Instructions.h
===================================================================
--- llvm/include/llvm/IR/Instructions.h
+++ llvm/include/llvm/IR/Instructions.h
@@ -105,6 +105,11 @@
     return cast<PointerType>(Instruction::getType());
   }
 
+  /// Return the address space for the allocation.
+  unsigned getAddressSpace() const {
+    return getType()->getAddressSpace();
+  }
+
   /// Get allocation size in bits. Returns None if size can't be determined,
   /// e.g. in case of a VLA.
   Optional<TypeSize> getAllocationSizeInBits(const DataLayout &DL) const;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D115895.394958.patch
Type: text/x-patch
Size: 4964 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211216/ddd7697b/attachment.bin>