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

412

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

412

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

412

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,
   // Special case intrinsics that "may have side effects" but can be deleted
   // 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;
 
+    if (II->isLifetimeStartOrEnd()) {
+      auto *Arg = II->getArgOperand(1);
       // Lifetime intrinsics are dead when their right-hand is undef.
-    if (II->isLifetimeStartOrEnd())
-      return isa<UndefValue>(II->getArgOperand(1));
+      if (isa<UndefValue>(Arg))
+        return true;
+      // If the right-hand is an alloc, global, or argument and the only uses
+      // are lifetime intrinsics then the intrinsics are dead.
+      if (isa<AllocaInst>(Arg) || isa<GlobalValue>(Arg) || isa<Argument>(Arg))
+        return llvm::all_of(Arg->uses(), [](Use &Use) {
+          if (IntrinsicInst *IntrinsicUse =
+                  dyn_cast<IntrinsicInst>(Use.getUser()))
+            return IntrinsicUse->isLifetimeStartOrEnd();
+          return false;
+        });
+      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/Analysis/BasicAA/modref.ll

 }
 
 define void @test3a(i8* %P, i8 %X) {
 ; CHECK-LABEL: @test3a
   %Y = add i8 %X, 1     ;; Dead, because the only use (the store) is dead.
 
   %P2 = getelementptr i8, i8* %P, i32 2
   store i8 %Y, i8* %P2
-; CHECK-NEXT: call void @llvm.lifetime.end
   call void @llvm.lifetime.end.p0i8(i64 10, i8* %P)
   ret void
 ; CHECK-NEXT: ret void
 }
 
 @G1 = external global i32
 @G2 = external global [4000 x i32]
 

llvm/test/Transforms/Attributor/memory_locations.ll

 ; CHECK-NEXT:    [[UNKNOWN:%.*]] = call i8* @inaccesible_argmem_only_decl(i8* noalias nocapture 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
 }
-; CHECK: Function Attrs: argmemonly nounwind willreturn
+
+; CHECK: Function Attrs: nofree {{(norecurse )?}}nosync nounwind readnone willreturn
 define void @callerE(i8* %arg) {
 ; CHECK-LABEL: define {{[^@]+}}@callerE
-; CHECK-SAME: (i8* nocapture [[ARG:%.*]])
-; CHECK-NEXT:    call void @llvm.lifetime.start.p0i8(i64 4, i8* nocapture [[ARG]])
+; CHECK-SAME: (i8* nocapture nofree readnone [[ARG:%.*]])
 ; CHECK-NEXT:    ret void
 ;
   call void @llvm.lifetime.start.p0i8(i64 4, i8* %arg)
   ret void
 }
 
 @G = external dso_local global i32, align 4
 

llvm/test/Transforms/DCE/basic.ll

 ; RUN: opt -debugify -dce -S < %s | FileCheck %s
 ; RUN: opt -passes='module(debugify),function(dce)' -S < %s | FileCheck %s
 
 ; CHECK-LABEL: @test
 define void @test() {
   %add = add i32 1, 2
 ; CHECK-NEXT: call void @llvm.dbg.value(metadata i32 1, metadata [[add:![0-9]+]], metadata !DIExpression(DW_OP_plus_uconst, 2, DW_OP_stack_value))
   %sub = sub i32 %add, 1
 ; CHECK-NEXT: call void @llvm.dbg.value(metadata i32 1, metadata [[sub:![0-9]+]], metadata !DIExpression(DW_OP_plus_uconst, 2, DW_OP_constu, 1, DW_OP_minus, DW_OP_stack_value))
 ; CHECK-NEXT: ret void
   ret void
 }
 
+declare void @llvm.lifetime.start.p0i8(i64, i8* nocapture) nounwind
+declare void @llvm.lifetime.end.p0i8(i64, i8* nocapture) nounwind
+
+; CHECK-LABEL: @test_lifetime_alloca
+define i32 @test_lifetime_alloca() {
+; Check that lifetime intrinsics are removed along with the pointer.
+; CHECK-NEXT: @llvm.dbg.value
+; CHECK-NEXT: ret i32 0
+; CHECK-NOT: llvm.lifetime.start
+; CHECK-NOT: llvm.lifetime.end
+  %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-LABEL: @test_lifetime_arg
+define i32 @test_lifetime_arg(i8*) {
+; Check that lifetime intrinsics are removed along with the pointer.
+; CHECK-NEXT: llvm.dbg.value
+; CHECK-NEXT: ret i32 0
+; CHECK-NOT: llvm.lifetime.start
+; CHECK-NOT: llvm.lifetime.end
+  call void @llvm.lifetime.start.p0i8(i64 -1, i8* %0)
+  call void @llvm.lifetime.end.p0i8(i64 -1, i8* %0)
+  ret i32 0
+}
+
+@glob = global i8 1
+
+; CHECK-LABEL: @test_lifetime_global
+define i32 @test_lifetime_global() {
+; Check that lifetime intrinsics are removed along with the pointer.
+; CHECK-NEXT: llvm.dbg.value
+; CHECK-NEXT: ret i32 0
+; CHECK-NOT: llvm.lifetime.start
+; CHECK-NOT: llvm.lifetime.end
+  call void @llvm.lifetime.start.p0i8(i64 -1, i8* @glob)
+  call void @llvm.lifetime.end.p0i8(i64 -1, i8* @glob)
+  ret i32 0
+}
+
+; CHECK-LABEL: @test_lifetime_bitcast
+define i32 @test_lifetime_bitcast(i32*) {
+; Check that lifetime intrinsics are NOT removed when the pointer is a bitcast.
+; It's not uncommon for two bitcasts to be made: one for lifetime, one for use.
+; TODO: Support the above case.
+; CHECK-NEXT: bitcast
+; CHECK-NEXT: llvm.dbg.value
+; CHECK-NEXT: llvm.lifetime.start
+; CHECK-NEXT: llvm.lifetime.end
+; CHECK-NEXT: ret i32 0
+  %2 = bitcast i32* %0 to i8*
+  call void @llvm.lifetime.start.p0i8(i64 -1, i8* %2)
+  call void @llvm.lifetime.end.p0i8(i64 -1, i8* %2)
+  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*)