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