rsmith created this revision.
rsmith added a reviewer: rjmccall.

When a non-extended temporary object is created in a conditional branch, the 
lifetime of that temporary ends outside the conditional (at the end of the 
full-expression). If we're inserting lifetime markers, this means we can end up 
generating

  if (some_cond) {
    lifetime.start(&tmp);
    Tmp::Tmp(&tmp);
  }
  // ...
  if (some_cond) {
    lifetime.end(&tmp);
  }

for a full-expression containing a subexpression of the form `some_cond ? 
Tmp().x : 0`. This patch moves the lifetime start for such a temporary out of 
the conditional branch so that we don't need to generate an additional basic 
block to hold the lifetime end marker.

This is disabled if we want precise lifetime markers (for asan's 
stack-use-after-scope checks) or of the temporary has a non-trivial destructor 
(in which case we'd generate an extra basic block anyway to hold the destructor 
call).


Repository:
  rC Clang

https://reviews.llvm.org/D50286

Files:
  lib/CodeGen/CGExpr.cpp
  test/CodeGenCXX/conditional-temporaries.cpp

Index: test/CodeGenCXX/conditional-temporaries.cpp
===================================================================
--- test/CodeGenCXX/conditional-temporaries.cpp
+++ test/CodeGenCXX/conditional-temporaries.cpp
@@ -1,6 +1,7 @@
 // REQUIRES: amdgpu-registered-target
-// RUN: %clang_cc1 -emit-llvm %s -o - -triple=x86_64-apple-darwin9 -O3 | FileCheck %s
-// RUN: %clang_cc1 -emit-llvm %s -o - -triple=amdgcn-amd-amdhsa -O3 | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm %s -o - -triple=x86_64-apple-darwin9 -O2 -disable-llvm-passes | FileCheck %s --check-prefixes=CHECK,CHECK-NOOPT
+// RUN: %clang_cc1 -emit-llvm %s -o - -triple=x86_64-apple-darwin9 -O2 | FileCheck %s --check-prefixes=CHECK,CHECK-OPT
+// RUN: %clang_cc1 -emit-llvm %s -o - -triple=amdgcn-amd-amdhsa -O2 | FileCheck %s --check-prefixes=CHECK,CHECK-OPT
 
 namespace {
 
@@ -38,20 +39,167 @@
 
 }
 
-// CHECK-LABEL: define i32 @_Z12getCtorCallsv()
+// CHECK-OPT-LABEL: define i32 @_Z12getCtorCallsv()
 int getCtorCalls() {
-  // CHECK: ret i32 5
+  // CHECK-OPT: ret i32 5
   return ctorcalls;
 }
 
-// CHECK-LABEL: define i32 @_Z12getDtorCallsv()
+// CHECK-OPT-LABEL: define i32 @_Z12getDtorCallsv()
 int getDtorCalls() {
-  // CHECK: ret i32 5
+  // CHECK-OPT: ret i32 5
   return dtorcalls;
 }
 
-// CHECK-LABEL: define zeroext i1 @_Z7successv()
+// CHECK-OPT-LABEL: define zeroext i1 @_Z7successv()
 bool success() {
-  // CHECK: ret i1 true
+  // CHECK-OPT: ret i1 true
   return ctorcalls == dtorcalls;
 }
+
+struct X { ~X(); int f(); };
+int g(int, int, int);
+// CHECK-LABEL: @_Z16lifetime_nontriv
+int lifetime_nontriv(bool cond) {
+  // CHECK-NOOPT: store i1 false,
+  // CHECK-NOOPT: store i1 false,
+  // CHECK-NOOPT: store i1 false,
+  // CHECK-NOOPT: store i1 false,
+  // CHECK-NOOPT: store i1 false,
+  // CHECK-NOOPT: store i1 false,
+  // CHECK-NOOPT: br i1
+  //
+  // CHECK-NOOPT: call void @llvm.lifetime.start
+  // CHECK-NOOPT: store i1 true,
+  // CHECK-NOOPT: store i1 true,
+  // CHECK-NOOPT: call i32 @_ZN1X1fEv(
+  // CHECK-NOOPT: call void @llvm.lifetime.start
+  // CHECK-NOOPT: store i1 true,
+  // CHECK-NOOPT: store i1 true,
+  // CHECK-NOOPT: call i32 @_ZN1X1fEv(
+  // CHECK-NOOPT: call void @llvm.lifetime.start
+  // CHECK-NOOPT: store i1 true,
+  // CHECK-NOOPT: store i1 true,
+  // CHECK-NOOPT: call i32 @_ZN1X1fEv(
+  // CHECK-NOOPT: call i32 @_Z1giii(
+  // CHECK-NOOPT: br label
+  //
+  // CHECK-NOOPT: call i32 @_Z1giii(i32 1, i32 2, i32 3)
+  // CHECK-NOOPT: br label
+  //
+  // CHECK-NOOPT: load i1,
+  // CHECK-NOOPT: br i1
+  // CHECK-NOOPT: call void @_ZN1XD1Ev(
+  // CHECK-NOOPT: br label
+  //
+  // CHECK-NOOPT: load i1,
+  // CHECK-NOOPT: br i1
+  // CHECK-NOOPT: call void @llvm.lifetime.end
+  // CHECK-NOOPT: br label
+  //
+  // CHECK-NOOPT: load i1,
+  // CHECK-NOOPT: br i1
+  // CHECK-NOOPT: call void @_ZN1XD1Ev(
+  // CHECK-NOOPT: br label
+  //
+  // CHECK-NOOPT: load i1,
+  // CHECK-NOOPT: br i1
+  // CHECK-NOOPT: call void @llvm.lifetime.end
+  // CHECK-NOOPT: br label
+  //
+  // CHECK-NOOPT: load i1,
+  // CHECK-NOOPT: br i1
+  // CHECK-NOOPT: call void @_ZN1XD1Ev(
+  // CHECK-NOOPT: br label
+  //
+  // CHECK-NOOPT: load i1,
+  // CHECK-NOOPT: br i1
+  // CHECK-NOOPT: call void @llvm.lifetime.end
+  // CHECK-NOOPT: br label
+  //
+  // CHECK-NOOPT: ret
+
+  // CHECK-OPT: br i1
+  //
+  // CHECK-OPT: call void @llvm.lifetime.start
+  // CHECK-OPT: call i32 @_ZN1X1fEv(
+  // CHECK-OPT: call void @llvm.lifetime.start
+  // CHECK-OPT: call i32 @_ZN1X1fEv(
+  // CHECK-OPT: call void @llvm.lifetime.start
+  // CHECK-OPT: call i32 @_ZN1X1fEv(
+  // CHECK-OPT: call i32 @_Z1giii(
+  // CHECK-OPT: call void @_ZN1XD1Ev(
+  // CHECK-OPT: call void @llvm.lifetime.end
+  // CHECK-OPT: call void @_ZN1XD1Ev(
+  // CHECK-OPT: call void @llvm.lifetime.end
+  // CHECK-OPT: call void @_ZN1XD1Ev(
+  // CHECK-OPT: call void @llvm.lifetime.end
+  // CHECK-OPT: br label
+  return cond ? g(X().f(), X().f(), X().f()) : g(1, 2, 3);
+}
+
+struct Y { int f(); };
+int g(int, int, int);
+// CHECK-LABEL: @_Z13lifetime_triv
+int lifetime_triv(bool cond) {
+  // CHECK-NOOPT: call void @llvm.lifetime.start
+  // CHECK-NOOPT: call void @llvm.lifetime.start
+  // CHECK-NOOPT: call void @llvm.lifetime.start
+  // CHECK-NOOPT: br i1
+  //
+  // CHECK-NOOPT: call i32 @_ZN1Y1fEv(
+  // CHECK-NOOPT: call i32 @_ZN1Y1fEv(
+  // CHECK-NOOPT: call i32 @_ZN1Y1fEv(
+  // CHECK-NOOPT: call i32 @_Z1giii(
+  // CHECK-NOOPT: br label
+  //
+  // CHECK-NOOPT: call i32 @_Z1giii(i32 1, i32 2, i32 3)
+  // CHECK-NOOPT: br label
+  //
+  // CHECK-NOOPT: call void @llvm.lifetime.end
+  // CHECK-NOOPT-NOT: br
+  // CHECK-NOOPT: call void @llvm.lifetime.end
+  // CHECK-NOOPT-NOT: br
+  // CHECK-NOOPT: call void @llvm.lifetime.end
+  //
+  // CHECK-NOOPT: ret
+
+  // FIXME: LLVM isn't smart enough to remove the lifetime markers from the
+  // g(1, 2, 3) path here.
+
+  // CHECK-OPT: call void @llvm.lifetime.start
+  // CHECK-OPT: call void @llvm.lifetime.start
+  // CHECK-OPT: call void @llvm.lifetime.start
+  // CHECK-OPT: br i1
+  //
+  // CHECK-OPT: call i32 @_ZN1Y1fEv(
+  // CHECK-OPT: call i32 @_ZN1Y1fEv(
+  // CHECK-OPT: call i32 @_ZN1Y1fEv(
+  // CHECK-OPT: call i32 @_Z1giii(
+  // CHECK-OPT: br label
+  //
+  // CHECK-OPT: call void @llvm.lifetime.end
+  // CHECK-OPT: call void @llvm.lifetime.end
+  // CHECK-OPT: call void @llvm.lifetime.end
+  return cond ? g(Y().f(), Y().f(), Y().f()) : g(1, 2, 3);
+}
+
+struct Z { ~Z() {} int f(); };
+int g(int, int, int);
+// CHECK-LABEL: @_Z22lifetime_nontriv_empty
+int lifetime_nontriv_empty(bool cond) {
+  // CHECK-OPT: br i1
+  //
+  // CHECK-OPT: call void @llvm.lifetime.start
+  // CHECK-OPT: call i32 @_ZN1Z1fEv(
+  // CHECK-OPT: call void @llvm.lifetime.start
+  // CHECK-OPT: call i32 @_ZN1Z1fEv(
+  // CHECK-OPT: call void @llvm.lifetime.start
+  // CHECK-OPT: call i32 @_ZN1Z1fEv(
+  // CHECK-OPT: call i32 @_Z1giii(
+  // CHECK-OPT: call void @llvm.lifetime.end
+  // CHECK-OPT: call void @llvm.lifetime.end
+  // CHECK-OPT: call void @llvm.lifetime.end
+  // CHECK-OPT: br label
+  return cond ? g(Z().f(), Z().f(), Z().f()) : g(1, 2, 3);
+}
Index: lib/CodeGen/CGExpr.cpp
===================================================================
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -498,18 +498,50 @@
   } else {
     switch (M->getStorageDuration()) {
     case SD_Automatic:
-    case SD_FullExpression:
       if (auto *Size = EmitLifetimeStart(
               CGM.getDataLayout().getTypeAllocSize(Alloca.getElementType()),
               Alloca.getPointer())) {
-        if (M->getStorageDuration() == SD_Automatic)
-          pushCleanupAfterFullExpr<CallLifetimeEnd>(NormalEHLifetimeMarker,
-                                                    Alloca, Size);
-        else
-          pushFullExprCleanup<CallLifetimeEnd>(NormalEHLifetimeMarker, Alloca,
-                                               Size);
+        pushCleanupAfterFullExpr<CallLifetimeEnd>(NormalEHLifetimeMarker,
+                                                  Alloca, Size);
       }
       break;
+
+    case SD_FullExpression: {
+      if (!ShouldEmitLifetimeMarkers)
+        break;
+
+      // Avoid creating a conditional cleanup just to hold an llvm.lifetime.end
+      // marker. Instead, start the lifetime of a conditional temporary earlier
+      // so that it's unconditional. Don't do this in ASan's use-after-scope
+      // mode so that it gets the more precise lifetime marks.
+      ConditionalEvaluation *OldConditional = nullptr;
+      CGBuilderTy::InsertPoint OldIP;
+      if (isInConditionalBranch() && !E->getType().isDestructedType() &&
+          CGM.getCodeGenOpts().OptimizationLevel > 0 &&
+          !CGM.getCodeGenOpts().SanitizeAddressUseAfterScope) {
+        OldConditional = OutermostConditional;
+        OutermostConditional = nullptr;
+
+        OldIP = Builder.saveIP();
+        llvm::BasicBlock *Block = OldConditional->getStartingBlock();
+        Builder.restoreIP(CGBuilderTy::InsertPoint(
+            Block, llvm::BasicBlock::iterator(Block->back())));
+      }
+
+      if (auto *Size = EmitLifetimeStart(
+              CGM.getDataLayout().getTypeAllocSize(Alloca.getElementType()),
+              Alloca.getPointer())) {
+        pushFullExprCleanup<CallLifetimeEnd>(NormalEHLifetimeMarker, Alloca,
+                                             Size);
+      }
+
+      if (OldConditional) {
+        OutermostConditional = OldConditional;
+        Builder.restoreIP(OldIP);
+      }
+      break;
+    }
+
     default:
       break;
     }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D50286: a... Richard Smith - zygoloid via Phabricator via cfe-commits

Reply via email to