Mark values as trivially dead when their only use is a start or end lifetime intrinsic.

[zoecarver]

Summary

If the only use of a value is a start or end lifetime intrinsic then mark the intrinsic as trivially dead. This should allow for that value to then be removed as well.

The only case where we may run into problems is the following:

%a = alloca
%e = get_element_ptr %a
call void @llvm.lifetime.start.p0i8(%e)

The element will no longer be marked as live. But, in all cases (I think) we should be able to remove both the intrinsics and the GEP. So, I don't think this will be an issue.

[zoecarver]

Phab used the description and title from my last patch. Sorry about that.

[jdoerfert]

llvm/lib/Transforms/Utils/Local.cpp

413

Style: Capital letter for variables.

Initially, I would prefer to add lifetime markers to the set of droppable uses and ask if the operand has only droppable uses. On second thought, we might not want to do this here because it would aggressively remove assumes. This patch could have a similar effect for useful lifetime markers, e.g. if you cast a pointer twice, once for lifetime once for use. Now I thought first that is unlikely but one of the tests already shows exactly this behavior: https://godbolt.org/z/oRLsrp Removing these lifetime markers and the bitcast is arguably not what we want. Put the call to bar in a loop and it becomes clear.

Multiple ways forward here, two come to mind first: (1) restrict the operand to be an alloca (argument, global, ...) or, (2) strip some casts and geps in both directions and then apply (1).

WDYT?

llvm/test/Transforms/DeadStoreElimination/lifetime.ll

15

CHECK-NOT instead?

[zoecarver]

@jdoerfert thanks for the review!

llvm/lib/Transforms/Utils/Local.cpp

413

That's a fair point which I didn't think about. I'll start with (1) and we can go from there.

llvm/test/Transforms/DeadStoreElimination/lifetime.ll

15

I thought about it when I made the change. My logic was that a DSE test shouldn't be testing the removal of lifetime intrinsics. That being said, it is a DSE lifetime test so it could probably go either way. I'll use CHECK-NOT.

[zoecarver]

• CHECK-NOT in DSE/lifetime.ll.
• Only mark alloc and global as dead.

[zoecarver]

• Diff from master

[zoecarver]

• Fix local variable spelling
• Add global test and arg test
• Remove unneeded changes to tests

[jdoerfert]

Minor things that can be addressed prior to the commit. Otherwise, LGTM. Thx.

llvm/lib/Transforms/Utils/Local.cpp

413

auto *Arg =

no else after return.

-II->getArgOperand(1)
+Arg

llvm/test/Transforms/DCE/basic.ll

53

Add one negative test, bitcast + lifetime marker, with a comment.

[zoecarver]

I had to revert this because of all the test failures outside llvm. I'll submit it for re-review before re-committing.

[zoecarver]

• Fix failing test

[zoecarver]

I'll re-commit tonight unless I hear otherwise.

llvm/lib/Transforms/Utils/Local.cpp

bool llvm::wouldInstructionBeTriviallyDead(Instruction *I,
   // when dead.
   if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(I)) {
     // Safe to delete llvm.stacksave and launder.invariant.group if dead.
     if (II->getIntrinsicID() == Intrinsic::stacksave ||
         II->getIntrinsicID() == Intrinsic::launder_invariant_group)
       return true;
 
     // Lifetime intrinsics are dead when their right-hand is undef.
-    if (II->isLifetimeStartOrEnd())
-      return isa<UndefValue>(II->getArgOperand(1)) ||
-             llvm::all_of(II->getArgOperand(1)->uses(), [](Use &use) {
-               if (IntrinsicInst *intrinsicUse =
-                       dyn_cast<IntrinsicInst>(use.getUser()))
-                 return intrinsicUse->isLifetimeStartOrEnd();
-               return false;
-             });
+    if (II->isLifetimeStartOrEnd()) {
+      auto Arg = II->getArgOperand(1);
+      if (isa<UndefValue>(Arg)) {
+        return true;
+      } else if (isa<AllocaInst>(Arg) || isa<GlobalValue>(Arg)) {
+        return llvm::all_of(II->getArgOperand(1)->uses(), [](Use &use) {
+      }
+      return false;
+    }
 
     // Assumptions are dead if their condition is trivially true.  Guards on
     // true are operationally no-ops.  In the future we can consider more
     // sophisticated tradeoffs for guards considering potential for check
     // widening, but for now we keep things simple.
     if ((II->getIntrinsicID() == Intrinsic::assume &&
          isAssumeWithEmptyBundle(*II)) ||
         II->getIntrinsicID() == Intrinsic::experimental_guard) {

llvm/test/Transforms/Attributor/memory_locations.ll

 ; CHECK-NEXT:    [[UNKNOWN:%.*]] = call i8* @inaccesible_argmem_only_decl(i8* noalias align 536870912 null)
 ; CHECK-NEXT:    store i8 0, i8* [[UNKNOWN]]
 ; CHECK-NEXT:    ret void
 ;
   %unknown = call i8* @inaccesible_argmem_only_decl(i8* null)
   store i8 0, i8* %unknown
   ret void
 }
-define void @callerE(i8* %arg) {
+define i8* @callerE(i8* %arg) {
 ; CHECK: Function Attrs: argmemonly nounwind willreturn
 ; CHECK-LABEL: define {{[^@]+}}@callerE
-; CHECK-SAME: (i8* nocapture [[ARG:%.*]])
+; CHECK-SAME: (i8* returned [[ARG:%.*]])
 ; CHECK-NEXT:    call void @llvm.lifetime.start.p0i8(i64 4, i8* nocapture [[ARG]])
-; CHECK-NEXT:    ret void
+; CHECK-NEXT:    ret i8* [[ARG]]
 ;
   call void @llvm.lifetime.start.p0i8(i64 4, i8* %arg)
-  ret void
+  ret i8* %arg
 }
 

llvm/test/Transforms/Coroutines/coro-split-02.ll

switch i8 %suspend, label %exit [
     i8 1, label %exit
   ]
 await.ready:
   %StrayCoroSave = call token @llvm.coro.save(i8* null)
   %val = load i32, i32* %Result.i19
   %cast = bitcast i32* %testval to i8*
   call void @llvm.lifetime.start.p0i8(i64 4, i8* %cast)
   call void @llvm.lifetime.end.p0i8(i64 4, i8*  %cast)
+  %dummy = load i8, i8* %cast
   call void @print(i32 %val)
   br label %exit
 exit:
   call i1 @llvm.coro.end(i8* null, i1 false)
   ret void
 }
 
 ; CHECK-LABEL: @a.resume(

llvm/test/Transforms/DCE/basic.ll

 
 declare void @llvm.lifetime.start.p0i8(i64, i8* nocapture) nounwind
 declare void @llvm.lifetime.end.p0i8(i64, i8* nocapture) nounwind
 
 ; CHECK-LABEL: @test_lifetime
 define i32 @test_lifetime() {
 ; Check that lifetime intrinsics are removed along with the pointer.
 ; CHECK-NEXT: @llvm.dbg.value
-; CHECK-NEXT: @llvm.dbg.value
 ; CHECK-NEXT: ret i32 0
 ; CHECK-NOT: llvm.lifetime.start
 ; CHECK-NOT: llvm.lifetime.end
-  %i = alloca i32, align 4
-  %i.ptr = bitcast i32* %i to i8*
-  call void @llvm.lifetime.start.p0i8(i64 -1, i8* %i.ptr)
-  call void @llvm.lifetime.end.p0i8(i64 -1, i8* %i.ptr)
+  %i = alloca i8, align 4
+  call void @llvm.lifetime.start.p0i8(i64 -1, i8* %i)
+  call void @llvm.lifetime.end.p0i8(i64 -1, i8* %i)
   ret i32 0
 }
 
 ; CHECK: [[add]] = !DILocalVariable
 ; CHECK: [[sub]] = !DILocalVariable

llvm/test/Transforms/DeadStoreElimination/lifetime.ll

 ; RUN: opt -S -basicaa -dse < %s | FileCheck %s
 
 target datalayout = "E-p:64:64:64-a0:0:8-f32:32:32-f64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-v64:64:64-v128:128:128"
 
 declare void @llvm.lifetime.start.p0i8(i64, i8* nocapture) nounwind
 declare void @llvm.lifetime.end.p0i8(i64, i8* nocapture) nounwind
 declare void @llvm.memset.p0i8.i8(i8* nocapture, i8, i8, i1) nounwind
 
 define void @test1() {
 ; CHECK-LABEL: @test1(
   %A = alloca i8
 
   store i8 0, i8* %A  ;; Written to by memset
   call void @llvm.lifetime.end.p0i8(i64 1, i8* %A)
-; CHECK: lifetime.end
+; CHECK-NOT: lifetime.end
 
   call void @llvm.memset.p0i8.i8(i8* %A, i8 0, i8 -1, i1 false)
 ; CHECK-NOT: memset
 
   ret void
 ; CHECK: ret void
 }
 
 define void @test2(i32* %P) {
 ; CHECK: test2
   %Q = getelementptr i32, i32* %P, i32 1
   %R = bitcast i32* %Q to i8*
   call void @llvm.lifetime.start.p0i8(i64 4, i8* %R)
-; CHECK: lifetime.start
   store i32 0, i32* %Q  ;; This store is dead.
 ; CHECK-NOT: store
   call void @llvm.lifetime.end.p0i8(i64 4, i8* %R)
-; CHECK: lifetime.end
   ret void
 }
 
 

llvm/test/Transforms/InstCombine/vararg.ll

-; RUN: opt < %s -instcombine -instcombine-infinite-loop-threshold=2 -S | FileCheck %s
+; RUN: opt < %s -instcombine -instcombine-infinite-loop-threshold=3 -S | FileCheck %s
 
 %struct.__va_list = type { i8*, i8*, i8*, i32, i32 }
 
 declare void @llvm.lifetime.start.p0i8(i64, i8* nocapture)
 declare void @llvm.lifetime.end.p0i8(i64, i8* nocapture)
 declare void @llvm.va_start(i8*)
 declare void @llvm.va_end(i8*)
 declare void @llvm.va_copy(i8*, i8*)