[llvm] [GVNSink] Do not sink lifetimes of different allocas (PR #149818)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-transforms

Author: Nikita Popov (nikic)

<details>
<summary>Changes</summary>

This was always undesirable, and after #<!-- -->149310 it is illegal and will result in a verifier error.

Fix this by moving SimplifyCFG's check for this into canReplaceOperandWithVariable(), so it's shared with GVNSink.

---
Full diff: https://github.com/llvm/llvm-project/pull/149818.diff


3 Files Affected:

- (modified) llvm/lib/Transforms/Utils/Local.cpp (+4) 
- (modified) llvm/lib/Transforms/Utils/SimplifyCFG.cpp (-10) 
- (added) llvm/test/Transforms/GVNSink/lifetime.ll (+77) 


``````````diff
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index 7f0c23bd24efb..6baa08d43a18b 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -3876,6 +3876,10 @@ bool llvm::canReplaceOperandWithVariable(const Instruction *I, unsigned OpIdx) {
   if (Op->isSwiftError())
     return false;
 
+  // Cannot replace alloca argument with phi/select.
+  if (I->isLifetimeStartOrEnd())
+    return false;
+
   // Early exit.
   if (!isa<Constant, InlineAsm>(Op))
     return true;
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index 75c96503d556d..94b0ab892f2dd 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -2227,16 +2227,6 @@ static bool canSinkInstructions(
       return I->getOperand(OI) == I0->getOperand(OI);
     };
     if (!all_of(Insts, SameAsI0)) {
-      // SROA can't speculate lifetime markers of selects/phis, and the
-      // backend may handle such lifetimes incorrectly as well (#104776).
-      // Don't sink lifetimes if it would introduce a phi on the pointer
-      // argument.
-      if (isa<LifetimeIntrinsic>(I0) && OI == 1 &&
-          any_of(Insts, [](const Instruction *I) {
-            return isa<AllocaInst>(I->getOperand(1)->stripPointerCasts());
-          }))
-        return false;
-
       if ((isa<Constant>(Op) && !replacingOperandWithVariableIsCheap(I0, OI)) ||
           !canReplaceOperandWithVariable(I0, OI))
         // We can't create a PHI from this GEP.
diff --git a/llvm/test/Transforms/GVNSink/lifetime.ll b/llvm/test/Transforms/GVNSink/lifetime.ll
new file mode 100644
index 0000000000000..1a8a69bb0986e
--- /dev/null
+++ b/llvm/test/Transforms/GVNSink/lifetime.ll
@@ -0,0 +1,77 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt -S -passes=gvn-sink < %s | FileCheck %s
+
+; Make sure we do not sink lifetime markers if this would introduce a
+; lifetime with non-alloca operand.
+
+define void @test_cant_sink(i1 %c) {
+; CHECK-LABEL: define void @test_cant_sink(
+; CHECK-SAME: i1 [[C:%.*]]) {
+; CHECK-NEXT:    [[A:%.*]] = alloca i8, align 1
+; CHECK-NEXT:    [[B:%.*]] = alloca i8, align 1
+; CHECK-NEXT:    call void @llvm.lifetime.start.p0(i64 1, ptr [[A]])
+; CHECK-NEXT:    call void @llvm.lifetime.start.p0(i64 1, ptr [[B]])
+; CHECK-NEXT:    br i1 [[C]], label %[[IF:.*]], label %[[ELSE:.*]]
+; CHECK:       [[IF]]:
+; CHECK-NEXT:    store i64 1, ptr [[A]], align 4
+; CHECK-NEXT:    call void @llvm.lifetime.end.p0(i64 1, ptr [[A]])
+; CHECK-NEXT:    br label %[[JOIN:.*]]
+; CHECK:       [[ELSE]]:
+; CHECK-NEXT:    store i64 1, ptr [[B]], align 4
+; CHECK-NEXT:    call void @llvm.lifetime.end.p0(i64 1, ptr [[B]])
+; CHECK-NEXT:    br label %[[JOIN]]
+; CHECK:       [[JOIN]]:
+; CHECK-NEXT:    ret void
+;
+  %a = alloca i8
+  %b = alloca i8
+  call void @llvm.lifetime.start(i64 1, ptr %a)
+  call void @llvm.lifetime.start(i64 1, ptr %b)
+  br i1 %c, label %if, label %else
+
+if:
+  store i64 1, ptr %a
+  call void @llvm.lifetime.end(i64 1, ptr %a)
+  br label %join
+
+else:
+  store i64 1, ptr %b
+  call void @llvm.lifetime.end(i64 1, ptr %b)
+  br label %join
+
+join:
+  ret void
+}
+
+define void @test_can_sink(i1 %c) {
+; CHECK-LABEL: define void @test_can_sink(
+; CHECK-SAME: i1 [[C:%.*]]) {
+; CHECK-NEXT:    [[A:%.*]] = alloca i8, align 1
+; CHECK-NEXT:    call void @llvm.lifetime.start.p0(i64 1, ptr [[A]])
+; CHECK-NEXT:    br i1 [[C]], label %[[IF:.*]], label %[[ELSE:.*]]
+; CHECK:       [[IF]]:
+; CHECK-NEXT:    br label %[[JOIN:.*]]
+; CHECK:       [[ELSE]]:
+; CHECK-NEXT:    br label %[[JOIN]]
+; CHECK:       [[JOIN]]:
+; CHECK-NEXT:    store i64 1, ptr [[A]], align 4
+; CHECK-NEXT:    call void @llvm.lifetime.end.p0(i64 1, ptr [[A]])
+; CHECK-NEXT:    ret void
+;
+  %a = alloca i8
+  call void @llvm.lifetime.start(i64 1, ptr %a)
+  br i1 %c, label %if, label %else
+
+if:
+  store i64 1, ptr %a
+  call void @llvm.lifetime.end(i64 1, ptr %a)
+  br label %join
+
+else:
+  store i64 1, ptr %a
+  call void @llvm.lifetime.end(i64 1, ptr %a)
+  br label %join
+
+join:
+  ret void
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/149818