From 692a147bf4339380ccfea7418cbedcea540cfaef Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov@redhat.com>
Date: Fri, 8 Apr 2022 15:16:03 +0200
Subject: [PATCH] [CGCall] Make findDominatingStoreToReturnValue() more robust

This was skipping specific lifetime + bitcast patterns, but with
opaque pointers the bitcast will not be present, and we did not
perform this fold.

Instead skip over lifetime.end and bitcasts generally, without
trying to correlate them.
---
 clang/lib/CodeGen/CGCall.cpp                    | 33 +++++++++----------------
 clang/test/CodeGen/dominating-store-to-return.c | 28 +++++++++++++++++++++
 2 files changed, 40 insertions(+), 21 deletions(-)
 create mode 100644 clang/test/CodeGen/dominating-store-to-return.c

diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index e2ffd57..c4d342e 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -3242,28 +3242,19 @@ static llvm::StoreInst *findDominatingStoreToReturnValue(CodeGenFunction &CGF) {
   if (!CGF.ReturnValue.getPointer()->hasOneUse()) {
     llvm::BasicBlock *IP = CGF.Builder.GetInsertBlock();
     if (IP->empty()) return nullptr;
-    llvm::Instruction *I = &IP->back();
-
-    // Skip lifetime markers
-    for (llvm::BasicBlock::reverse_iterator II = IP->rbegin(),
-                                            IE = IP->rend();
-         II != IE; ++II) {
-      if (llvm::IntrinsicInst *Intrinsic =
-              dyn_cast<llvm::IntrinsicInst>(&*II)) {
-        if (Intrinsic->getIntrinsicID() == llvm::Intrinsic::lifetime_end) {
-          const llvm::Value *CastAddr = Intrinsic->getArgOperand(1);
-          ++II;
-          if (II == IE)
-            break;
-          if (isa<llvm::BitCastInst>(&*II) && (CastAddr == &*II))
-            continue;
-        }
-      }
-      I = &*II;
-      break;
-    }
 
-    return GetStoreIfValid(I);
+    // Look at directly preceding instruction, skipping bitcasts and lifetime
+    // markers.
+    for (llvm::Instruction &I : make_range(IP->rbegin(), IP->rend())) {
+      if (isa<llvm::BitCastInst>(&I))
+        continue;
+      if (auto *II = dyn_cast<llvm::IntrinsicInst>(&I))
+        if (II->getIntrinsicID() == llvm::Intrinsic::lifetime_end)
+          continue;
+
+      return GetStoreIfValid(&I);
+    }
+    return nullptr;
   }
 
   llvm::StoreInst *store =
diff --git a/clang/test/CodeGen/dominating-store-to-return.c b/clang/test/CodeGen/dominating-store-to-return.c
new file mode 100644
index 0000000..42595c5
--- /dev/null
+++ b/clang/test/CodeGen/dominating-store-to-return.c
@@ -0,0 +1,28 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -opaque-pointers -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefix=NO-LIFETIME
+// RUN: %clang_cc1 -opaque-pointers -O1 -disable-llvm-optzns -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefix=LIFETIME
+
+// NO-LIFETIME-LABEL: @main(
+// NO-LIFETIME-NEXT:  entry:
+// NO-LIFETIME-NEXT:    [[RETVAL:%.*]] = alloca i32, align 4
+// NO-LIFETIME-NEXT:    [[FOO:%.*]] = alloca i32, align 4
+// NO-LIFETIME-NEXT:    [[FOO2:%.*]] = alloca i32, align 4
+// NO-LIFETIME-NEXT:    store i32 0, ptr [[RETVAL]], align 4
+// NO-LIFETIME-NEXT:    ret i32 0
+//
+// LIFETIME-LABEL: @main(
+// LIFETIME-NEXT:  entry:
+// LIFETIME-NEXT:    [[RETVAL:%.*]] = alloca i32, align 4
+// LIFETIME-NEXT:    [[FOO:%.*]] = alloca i32, align 4
+// LIFETIME-NEXT:    [[FOO2:%.*]] = alloca i32, align 4
+// LIFETIME-NEXT:    store i32 0, ptr [[RETVAL]], align 4
+// LIFETIME-NEXT:    call void @llvm.lifetime.start.p0(i64 4, ptr [[FOO]]) #[[ATTR2:[0-9]+]]
+// LIFETIME-NEXT:    call void @llvm.lifetime.start.p0(i64 4, ptr [[FOO2]]) #[[ATTR2]]
+// LIFETIME-NEXT:    call void @llvm.lifetime.end.p0(i64 4, ptr [[FOO2]]) #[[ATTR2]]
+// LIFETIME-NEXT:    call void @llvm.lifetime.end.p0(i64 4, ptr [[FOO]]) #[[ATTR2]]
+// LIFETIME-NEXT:    ret i32 0
+//
+int main() {
+  unsigned foo, foo2;
+  return 0;
+}
-- 
2.7.4