diff options
| -rw-r--r-- | clang/lib/CodeGen/CGExprAgg.cpp | 98 | ||||
| -rw-r--r-- | clang/test/CodeGenObjC/arc.m | 31 |
2 files changed, 82 insertions, 47 deletions
diff --git a/clang/lib/CodeGen/CGExprAgg.cpp b/clang/lib/CodeGen/CGExprAgg.cpp index 394336ee122..5613f8d9700 100644 --- a/clang/lib/CodeGen/CGExprAgg.cpp +++ b/clang/lib/CodeGen/CGExprAgg.cpp @@ -37,23 +37,6 @@ class AggExprEmitter : public StmtVisitor<AggExprEmitter> { AggValueSlot Dest; bool IsResultUnused; - /// We want to use 'dest' as the return slot except under two - /// conditions: - /// - The destination slot requires garbage collection, so we - /// need to use the GC API. - /// - The destination slot is potentially aliased. - bool shouldUseDestForReturnSlot() const { - return !(Dest.requiresGCollection() || Dest.isPotentiallyAliased()); - } - - ReturnValueSlot getReturnValueSlot() const { - if (!shouldUseDestForReturnSlot()) - return ReturnValueSlot(); - - return ReturnValueSlot(Dest.getAddress(), Dest.isVolatile(), - IsResultUnused); - } - AggValueSlot EnsureSlot(QualType T) { if (!Dest.isIgnored()) return Dest; return CGF.CreateAggTemp(T, "agg.tmp.ensured"); @@ -63,6 +46,15 @@ class AggExprEmitter : public StmtVisitor<AggExprEmitter> { Dest = CGF.CreateAggTemp(T, "agg.tmp.ensured"); } + // Calls `Fn` with a valid return value slot, potentially creating a temporary + // to do so. If a temporary is created, an appropriate copy into `Dest` will + // be emitted. + // + // The given function should take a ReturnValueSlot, and return an RValue that + // points to said slot. + void withReturnValueSlot(const Expr *E, + llvm::function_ref<RValue(ReturnValueSlot)> Fn); + public: AggExprEmitter(CodeGenFunction &cgf, AggValueSlot Dest, bool IsResultUnused) : CGF(cgf), Builder(CGF.Builder), Dest(Dest), @@ -242,34 +234,44 @@ bool AggExprEmitter::TypeRequiresGCollection(QualType T) { return Record->hasObjectMember(); } -/// \brief Perform the final move to DestPtr if for some reason -/// getReturnValueSlot() didn't use it directly. -/// -/// The idea is that you do something like this: -/// RValue Result = EmitSomething(..., getReturnValueSlot()); -/// EmitMoveFromReturnSlot(E, Result); -/// -/// If nothing interferes, this will cause the result to be emitted -/// directly into the return value slot. Otherwise, a final move -/// will be performed. -void AggExprEmitter::EmitMoveFromReturnSlot(const Expr *E, RValue src) { - // Push destructor if the result is ignored and the type is a C struct that - // is non-trivial to destroy. - QualType Ty = E->getType(); - if (Dest.isIgnored() && - Ty.isDestructedType() == QualType::DK_nontrivial_c_struct) - CGF.pushDestroy(Ty.isDestructedType(), src.getAggregateAddress(), Ty); - - if (shouldUseDestForReturnSlot()) { - // Logically, Dest.getAddr() should equal Src.getAggregateAddr(). - // The possibility of undef rvalues complicates that a lot, - // though, so we can't really assert. - return; +void AggExprEmitter::withReturnValueSlot( + const Expr *E, llvm::function_ref<RValue(ReturnValueSlot)> EmitCall) { + QualType RetTy = E->getType(); + bool RequiresDestruction = + Dest.isIgnored() && + RetTy.isDestructedType() == QualType::DK_nontrivial_c_struct; + + // If it makes no observable difference, save a memcpy + temporary. + // + // We need to always provide our own temporary if destruction is required. + // Otherwise, EmitCall will emit its own, notice that it's "unused", and end + // its lifetime before we have the chance to emit a proper destructor call. + bool UseTemp = Dest.isPotentiallyAliased() || Dest.requiresGCollection() || + (RequiresDestruction && !Dest.getAddress().isValid()); + + Address RetAddr = Address::invalid(); + if (!UseTemp) { + RetAddr = Dest.getAddress(); + } else { + RetAddr = CGF.CreateMemTemp(RetTy); + uint64_t Size = + CGF.CGM.getDataLayout().getTypeAllocSize(CGF.ConvertTypeForMem(RetTy)); + if (llvm::Value *LifetimeSizePtr = + CGF.EmitLifetimeStart(Size, RetAddr.getPointer())) + CGF.pushFullExprCleanup<CodeGenFunction::CallLifetimeEnd>( + NormalEHLifetimeMarker, RetAddr, LifetimeSizePtr); } - // Otherwise, copy from there to the destination. - assert(Dest.getPointer() != src.getAggregatePointer()); - EmitFinalDestCopy(E->getType(), src); + RValue Src = + EmitCall(ReturnValueSlot(RetAddr, Dest.isVolatile(), IsResultUnused)); + + if (RequiresDestruction) + CGF.pushDestroy(RetTy.isDestructedType(), Src.getAggregateAddress(), RetTy); + + if (UseTemp) { + assert(Dest.getPointer() != Src.getAggregatePointer()); + EmitFinalDestCopy(E->getType(), Src); + } } /// EmitFinalDestCopy - Perform the final copy to DestPtr, if desired. @@ -828,13 +830,15 @@ void AggExprEmitter::VisitCallExpr(const CallExpr *E) { return; } - RValue RV = CGF.EmitCallExpr(E, getReturnValueSlot()); - EmitMoveFromReturnSlot(E, RV); + withReturnValueSlot(E, [&](ReturnValueSlot Slot) { + return CGF.EmitCallExpr(E, Slot); + }); } void AggExprEmitter::VisitObjCMessageExpr(ObjCMessageExpr *E) { - RValue RV = CGF.EmitObjCMessageExpr(E, getReturnValueSlot()); - EmitMoveFromReturnSlot(E, RV); + withReturnValueSlot(E, [&](ReturnValueSlot Slot) { + return CGF.EmitObjCMessageExpr(E, Slot); + }); } void AggExprEmitter::VisitBinComma(const BinaryOperator *E) { diff --git a/clang/test/CodeGenObjC/arc.m b/clang/test/CodeGenObjC/arc.m index 7871369413e..3243952a36b 100644 --- a/clang/test/CodeGenObjC/arc.m +++ b/clang/test/CodeGenObjC/arc.m @@ -1526,6 +1526,37 @@ void test70(id i) { }; } +// Be sure that we emit lifetime intrinsics only after dtors +struct AggDtor { + char cs[40]; + id x; +}; + +struct AggDtor getAggDtor(void); + +// CHECK-LABEL: define void @test71 +void test71(void) { + // FIXME: It would be nice if the __destructor_8_s40 for the first call (and + // the following lifetime.end) came before the second call. + // + // CHECK: %[[T:[^ ]+]] = bitcast %struct.AggDtor* %[[TMP1:[^ ]+]] to i8* + // CHECK: call void @llvm.lifetime.start.p0i8({{[^,]+}}, i8* %[[T]]) + // CHECK: call void @getAggDtor(%struct.AggDtor* sret %[[TMP1]]) + // CHECK: %[[T:[^ ]+]] = bitcast %struct.AggDtor* %[[TMP2:[^ ]+]] to i8* + // CHECK: call void @llvm.lifetime.start.p0i8({{[^,]+}}, i8* %[[T]]) + // CHECK: call void @getAggDtor(%struct.AggDtor* sret %[[TMP2]]) + // CHECK: %[[T:[^ ]+]] = bitcast %struct.AggDtor* %[[TMP2]] to i8** + // CHECK: call void @__destructor_8_s40(i8** %[[T]]) + // CHECK: %[[T:[^ ]+]] = bitcast %struct.AggDtor* %[[TMP2:[^ ]+]] to i8* + // CHECK: call void @llvm.lifetime.end.p0i8({{[^,]+}}, i8* %[[T]]) + // CHECK: %[[T:[^ ]+]] = bitcast %struct.AggDtor* %[[TMP1]] to i8** + // CHECK: call void @__destructor_8_s40(i8** %[[T]]) + // CHECK: %[[T:[^ ]+]] = bitcast %struct.AggDtor* %[[TMP1:[^ ]+]] to i8* + // CHECK: call void @llvm.lifetime.end.p0i8({{[^,]+}}, i8* %[[T]]) + getAggDtor(); + getAggDtor(); +} + // ARC-ALIEN: attributes [[NLB]] = { nonlazybind } // ARC-NATIVE: attributes [[NLB]] = { nonlazybind } // CHECK: attributes [[NUW]] = { nounwind } |
