[PATCH] D79355: Mark values as trivially dead when their only use is a start or end lifetime intrinsic.

[Zoe Carver via Phabricator via llvm-commits]

zoecarver marked 2 inline comments as done.
zoecarver added a comment.

@jdoerfert thanks for the review!



================
Comment at: llvm/lib/Transforms/Utils/Local.cpp:415
+               return false;
+             });
 
----------------
jdoerfert wrote:
> 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?
That's a fair point which I didn't think about. I'll start with (1) and we can go from there. 


================
Comment at: llvm/test/Transforms/DeadStoreElimination/lifetime.ll:15
   call void @llvm.lifetime.end.p0i8(i64 1, i8* %A)
-; CHECK: lifetime.end
 
----------------
jdoerfert wrote:
> CHECK-NOT instead?
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`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79355/new/

https://reviews.llvm.org/D79355