Enabling the copy-constant-to-alloca optimization in more instances

[mfawaz]

Summary

This patch allows lifetime calls to be ignored (and later erased) if we
know that the copy-constant-to-alloca optimization is going to happen.
The case that is missed is when the global variable is in a different address
space than the alloca (as shown in the example added to the lit test.)

This used to work before https://github.com/llvm/llvm-project/commit/6da31fa4a61d68af21dfa1e144e726ed6d77903e

[arsenm]

LGTM but I don’t see how this patch is related to the address spaces

[mfawaz]

In D106573#2897217, @arsenm wrote:

LGTM but I don’t see how this patch is related to the address spaces

Thanks Matt. There reason I'm mentioning address spaces is because the case where address spaces are the same is handled by an earlier section of the code, and this patch seems to only be relevant when address spaces are different.

[andrew.w.kaylor]

lgtm

[jeroen.dobbelaere]

This patch triggers a (latent) infinite loop problem in instcombine when the memcpy cannot be removed.
It can be seen when replacing the 'i1 false' of the llvm.memcpy in test11 with a 'i1 true', making the memcpy volatile.

[mfawaz]

In D106573#2909860, @jeroen.dobbelaere wrote:

This patch triggers a (latent) infinite loop problem in instcombine when the memcpy cannot be removed.
It can be seen when replacing the 'i1 false' of the llvm.memcpy in test11 with a 'i1 true', making the memcpy volatile.

Thank you pointing this out. I have a fix that I am currently testing.

[mfawaz]

In D106573#2909860, @jeroen.dobbelaere wrote:

This patch triggers a (latent) infinite loop problem in instcombine when the memcpy cannot be removed.
It can be seen when replacing the 'i1 false' of the llvm.memcpy in test11 with a 'i1 true', making the memcpy volatile.

New differential with the fix here: https://reviews.llvm.org/D106950

llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp

if (LoadInst *Load = dyn_cast<LoadInst>(Inst)) {
         return false;
       Worklist.insert(Load);
     } else if (isa<GetElementPtrInst>(Inst) || isa<BitCastInst>(Inst)) {
       Worklist.insert(Inst);
       if (!collectUsers(*Inst))
         return false;
     } else if (isa<MemTransferInst>(Inst)) {
       Worklist.insert(Inst);
+    } else if (Inst->isLifetimeStartOrEnd()) {
+      continue;
     } else {
       LLVM_DEBUG(dbgs() << "Cannot handle pointer user: " << *U << '\n');
       return false;
     }
   }
 
   return true;
 }

llvm/test/Transforms/InstCombine/memcpy-from-global.ll

 declare void @llvm.memcpy.p0i8.p1i8.i64(i8* nocapture, i8 addrspace(1)* nocapture, i64, i1) nounwind
 declare void @llvm.memcpy.p1i8.p1i8.i64(i8 addrspace(1)* nocapture, i8 addrspace(1)* nocapture, i64, i1) nounwind
 
 %T = type { i8, [123 x i8] }
 %U = type { i32, i32, i32, i32, i32 }
 
 @G = constant %T {i8 1, [123 x i8] zeroinitializer }
 @H = constant [2 x %U] zeroinitializer, align 16
+@I = internal addrspace(1) constant [4 x float] zeroinitializer , align 4
 
 define void @test2() {
 ; CHECK-LABEL: @test2(
 ; CHECK-NEXT:    [[B1:%.*]] = alloca [124 x i8], align 8
 ; CHECK-NEXT:    [[B1_SUB:%.*]] = getelementptr inbounds [124 x i8], [124 x i8]* [[B1]], i64 0, i64 0
 ; CHECK-NEXT:    call void @llvm.memcpy.p0i8.p0i8.i64(i8* noundef nonnull align 8 dereferenceable(124) [[B1_SUB]], i8* noundef nonnull align 16 dereferenceable(124) getelementptr inbounds (%T, %T* @G, i64 0, i32 0), i64 124, i1 false)
 ; CHECK-NEXT:    call void @bar(i8* nonnull [[B1_SUB]])
 ; CHECK-NEXT:    ret void
entry:
   %cc = alloca [3 x i8], align 1
   %cc.0..sroa_idx = getelementptr inbounds [3 x i8], [3 x i8]* %cc, i64 0, i64 0
   %arraydecay = getelementptr inbounds [3 x i8], [3 x i8]* %cc, i32 0, i32 0
   call void @llvm.memcpy.p0i8.p0i8.i64(i8* %arraydecay, i8* getelementptr inbounds ([3 x i8], [3 x i8]* @_ZL3KKK, i32 0, i32 0), i64 3, i1 false)
   call void @llvm.memcpy.p0i8.p0i8.i64(i8* getelementptr inbounds ([1000000 x i8], [1000000 x i8]* @bbb, i32 0, i32 0), i8* %arraydecay, i64 3, i1 false)
   ret void
 }
 
+; Should replace alloca with global even when the global is in a different address space
+define float @test11(i64 %i) {
+; CHECK-LABEL: @test11(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[GEP:%.*]] = getelementptr [4 x float], [4 x float] addrspace(1)* @I, i64 0, i64 %i
+; CHECK-NEXT: [[LD:%.*]] = load float, float addrspace(1)* [[GEP]]
+; CHECK-NEXT: ret float [[LD]]
+
+entry:
+  %a = alloca [4 x float], align 4
+  %b = bitcast [4 x float]* %a to i8*
+  call void @llvm.lifetime.start.p0i8(i64 16, i8* %b)
+  call void @llvm.memcpy.p0i8.p1i8.i64(i8* align 4 %b, i8 addrspace(1)* align 4 bitcast ([4 x float] addrspace(1)* @I to i8 addrspace(1)*), i64 16, i1 false)
+  %g = getelementptr inbounds [4 x float], [4 x float]* %a, i64 0, i64 %i
+  %r = load float, float* %g, align 4
+  ret float %r
+}
+
 attributes #0 = { null_pointer_is_valid }