ahatanak updated this revision to Diff 457620. ahatanak added a comment. Call the move assignment operator in the setter instead of calling the copy assignment operator and the destructor.
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131701/new/ https://reviews.llvm.org/D131701 Files: clang/include/clang/AST/Decl.h clang/lib/AST/Decl.cpp clang/lib/CodeGen/CGDecl.cpp clang/lib/CodeGen/CGObjC.cpp clang/test/CodeGenObjC/nontrivial-c-struct-property.m
Index: clang/test/CodeGenObjC/nontrivial-c-struct-property.m =================================================================== --- /dev/null +++ clang/test/CodeGenObjC/nontrivial-c-struct-property.m @@ -0,0 +1,70 @@ +// RUN: %clang_cc1 -triple arm64-apple-ios11 -fobjc-arc -emit-llvm -o - %s | FileCheck %s + +typedef struct { + id x; +} S0; + +@interface C { + S0 _p1; +} +@property(nonatomic) S0 nonatomic; +@property S0 atomic0; +@property S0 p1; +-(S0)p1; +-(void)setP1:(S0)s0; +@end + +@implementation C +-(S0)p1 { + return _p1; +} +-(void)setP1:(S0)s0 { + _p1 = s0; +} +@end + +// CHECK: %[[STRUCT_S0:.*]] = type { ptr } + +// Check that parameters of user-defined setters are destructed. + +// CHECK-LABEL: define internal void @"\01-[C setP1:]"( +// CHECK: %[[S0:.*]] = alloca %[[STRUCT_S0]], align 8 +// CHECK: call void @__copy_assignment_8_8_s0(ptr %{{.*}}, ptr %[[S0]]) +// CHECK: call void @__destructor_8_s0(ptr %[[S0]]) + +// CHECK: define internal i64 @"\01-[C nonatomic]"(ptr noundef %[[SELF:.*]], {{.*}}) +// CHECK: %[[RETVAL:.*]] = alloca %[[STRUCT_S0]], align 8 +// CHECK: %[[SELF_ADDR:.*]] = alloca ptr, align 8 +// CHECK: store ptr %[[SELF]], ptr %[[SELF_ADDR]], align 8 +// CHECK: %[[V0:.*]] = load ptr, ptr %[[SELF_ADDR]], align 8 +// CHECK: %[[IVAR:.*]] = load i32, ptr @"OBJC_IVAR_$_C._nonatomic", align 8 +// CHECK: %[[IVAR_CONV:.*]] = sext i32 %[[IVAR]] to i64 +// CHECK: %[[ADD_PTR:.*]] = getelementptr inbounds i8, ptr %[[V0]], i64 %[[IVAR_CONV]] +// CHECK: call void @__copy_constructor_8_8_s0(ptr %[[RETVAL]], ptr %[[ADD_PTR]]) +// CHECK-NOT: call +// CHECK: ret i64 + +// CHECK: define internal void @"\01-[C setNonatomic:]"(ptr noundef %[[SELF:.*]], {{.*}}, i64 %[[NONATOMIC_COERCE:.*]]) +// CHECK: %[[NONATOMIC:.*]] = alloca %[[STRUCT_S0]], align 8 +// CHECK: %[[SELF_ADDR:.*]] = alloca ptr, align 8 +// CHECK: %[[COERCE_DIVE:.*]] = getelementptr inbounds %[[STRUCT_S0]], ptr %[[NONATOMIC]], i32 0, i32 0 +// CHECK: %[[COERCE_VAL_IP:.*]] = inttoptr i64 %[[NONATOMIC_COERCE]] to ptr +// CHECK: store ptr %[[COERCE_VAL_IP]], ptr %[[COERCE_DIVE]], align 8 +// CHECK: store ptr %[[SELF]], ptr %[[SELF_ADDR]], align 8 +// CHECK: %[[V0:.*]] = load ptr, ptr %[[SELF_ADDR]], align 8 +// CHECK: %[[IVAR:.*]] = load i32, ptr @"OBJC_IVAR_$_C._nonatomic", align 8 +// CHECK: %[[IVAR_CONV:.*]] = sext i32 %[[IVAR]] to i64 +// CHECK: %[[ADD_PTR:.*]] = getelementptr inbounds i8, ptr %[[V0]], i64 %[[IVAR_CONV]] +// CHECK: call void @__move_assignment_8_8_s0(ptr %[[ADD_PTR]], ptr %[[NONATOMIC]]) +// CHECK-NOT: call +// CHECK: ret void + +// CHECK-LABEL: define internal i64 @"\01-[C atomic0]"( +// CHECK: call void @objc_copyCppObjectAtomic({{.*}}, {{.*}}, ptr noundef @__copy_constructor_8_8_s0) +// CHECK-NOT: call +// CHECK: ret i64 + +// CHECK-LABEL: define internal void @"\01-[C setAtomic0:]"( +// CHECK: call void @objc_copyCppObjectAtomic({{.*}}, {{.*}}, ptr noundef @__move_assignment_8_8_s0) +// CHECK-NOT: call +// CHECK: ret void Index: clang/lib/CodeGen/CGObjC.cpp =================================================================== --- clang/lib/CodeGen/CGObjC.cpp +++ clang/lib/CodeGen/CGObjC.cpp @@ -22,6 +22,7 @@ #include "clang/AST/StmtObjC.h" #include "clang/Basic/Diagnostic.h" #include "clang/CodeGen/CGFunctionInfo.h" +#include "clang/CodeGen/CodeGenABITypes.h" #include "llvm/ADT/STLExtras.h" #include "llvm/Analysis/ObjCARCUtil.h" #include "llvm/BinaryFormat/MachO.h" @@ -1115,6 +1116,23 @@ const ObjCPropertyImplDecl *propImpl, const ObjCMethodDecl *GetterMethodDecl, llvm::Constant *AtomicHelperFn) { + + ObjCIvarDecl *ivar = propImpl->getPropertyIvarDecl(); + + if (ivar->getType().isNonTrivialToPrimitiveCopy() == QualType::PCK_Struct) { + if (!AtomicHelperFn) { + LValue Src = + EmitLValueForIvar(TypeOfSelfObject(), LoadObjCSelf(), ivar, 0); + LValue Dst = MakeAddrLValue(ReturnValue, ivar->getType()); + callCStructCopyConstructor(Dst, Src); + } else { + ObjCIvarDecl *ivar = propImpl->getPropertyIvarDecl(); + emitCPPObjectAtomicGetterCall(*this, ReturnValue.getPointer(), ivar, + AtomicHelperFn); + } + return; + } + // If there's a non-trivial 'get' expression, we just have to emit that. if (!hasTrivialGetExpr(propImpl)) { if (!AtomicHelperFn) { @@ -1135,8 +1153,6 @@ QualType propType = prop->getType(); ObjCMethodDecl *getterMethod = propImpl->getGetterMethodDecl(); - ObjCIvarDecl *ivar = propImpl->getPropertyIvarDecl(); - // Pick an implementation strategy. PropertyImplStrategy strategy(CGM, propImpl); switch (strategy.getKind()) { @@ -1404,6 +1420,22 @@ ObjCIvarDecl *ivar = propImpl->getPropertyIvarDecl(); ObjCMethodDecl *setterMethod = propImpl->getSetterMethodDecl(); + if (ivar->getType().isNonTrivialToPrimitiveCopy() == QualType::PCK_Struct) { + if (!AtomicHelperFn) { + // Call the move assignment operator instead of calling the copy + // assignment operator and destructor. + LValue Dst = EmitLValueForIvar(TypeOfSelfObject(), LoadObjCSelf(), ivar, + /*quals*/ 0); + LValue Src = MakeAddrLValue( + GetAddrOfLocalVar(*setterMethod->param_begin()), ivar->getType()); + callCStructMoveAssignmentOperator(Dst, Src); + } else { + // If atomic, assignment is called via a locking api. + emitCPPObjectAtomicSetterCall(*this, setterMethod, ivar, AtomicHelperFn); + } + return; + } + // Just use the setter expression if Sema gave us one and it's // non-trivial. if (!hasTrivialSetExpr(propImpl)) { @@ -3661,15 +3693,27 @@ llvm::Constant * CodeGenFunction::GenerateObjCAtomicSetterCopyHelperFunction( const ObjCPropertyImplDecl *PID) { + const ObjCPropertyDecl *PD = PID->getPropertyDecl(); + if ((!(PD->getPropertyAttributes() & ObjCPropertyAttribute::kind_atomic))) + return nullptr; + + QualType Ty = PID->getPropertyIvarDecl()->getType(); + ASTContext &C = getContext(); + + if (Ty.isNonTrivialToPrimitiveCopy() == QualType::PCK_Struct) { + // Call the move assignment operator instead of calling the copy assignment + // operator and destructor. + CharUnits Alignment = C.getTypeAlignInChars(Ty); + llvm::Constant *Fn = getNonTrivialCStructMoveAssignmentOperator( + CGM, Alignment, Alignment, Ty.isVolatileQualified(), Ty); + return llvm::ConstantExpr::getBitCast(Fn, VoidPtrTy); + } + if (!getLangOpts().CPlusPlus || !getLangOpts().ObjCRuntime.hasAtomicCopyHelper()) return nullptr; - QualType Ty = PID->getPropertyIvarDecl()->getType(); if (!Ty->isRecordType()) return nullptr; - const ObjCPropertyDecl *PD = PID->getPropertyDecl(); - if ((!(PD->getPropertyAttributes() & ObjCPropertyAttribute::kind_atomic))) - return nullptr; llvm::Constant *HelperFn = nullptr; if (hasTrivialSetExpr(PID)) return nullptr; @@ -3677,7 +3721,6 @@ if ((HelperFn = CGM.getAtomicSetterHelperFnMap(Ty))) return HelperFn; - ASTContext &C = getContext(); IdentifierInfo *II = &CGM.getContext().Idents.get("__assign_helper_atomic_property_"); @@ -3748,18 +3791,27 @@ return HelperFn; } -llvm::Constant * -CodeGenFunction::GenerateObjCAtomicGetterCopyHelperFunction( - const ObjCPropertyImplDecl *PID) { +llvm::Constant *CodeGenFunction::GenerateObjCAtomicGetterCopyHelperFunction( + const ObjCPropertyImplDecl *PID) { + const ObjCPropertyDecl *PD = PID->getPropertyDecl(); + if ((!(PD->getPropertyAttributes() & ObjCPropertyAttribute::kind_atomic))) + return nullptr; + + QualType Ty = PD->getType(); + ASTContext &C = getContext(); + + if (Ty.isNonTrivialToPrimitiveCopy() == QualType::PCK_Struct) { + CharUnits Alignment = C.getTypeAlignInChars(Ty); + llvm::Constant *Fn = getNonTrivialCStructCopyConstructor( + CGM, Alignment, Alignment, Ty.isVolatileQualified(), Ty); + return llvm::ConstantExpr::getBitCast(Fn, VoidPtrTy); + } + if (!getLangOpts().CPlusPlus || !getLangOpts().ObjCRuntime.hasAtomicCopyHelper()) return nullptr; - const ObjCPropertyDecl *PD = PID->getPropertyDecl(); - QualType Ty = PD->getType(); if (!Ty->isRecordType()) return nullptr; - if ((!(PD->getPropertyAttributes() & ObjCPropertyAttribute::kind_atomic))) - return nullptr; llvm::Constant *HelperFn = nullptr; if (hasTrivialGetExpr(PID)) return nullptr; @@ -3767,7 +3819,6 @@ if ((HelperFn = CGM.getAtomicGetterHelperFnMap(Ty))) return HelperFn; - ASTContext &C = getContext(); IdentifierInfo *II = &CGM.getContext().Idents.get("__copy_helper_atomic_property_"); Index: clang/lib/CodeGen/CGDecl.cpp =================================================================== --- clang/lib/CodeGen/CGDecl.cpp +++ clang/lib/CodeGen/CGDecl.cpp @@ -2505,9 +2505,16 @@ assert((DtorKind == QualType::DK_cxx_destructor || DtorKind == QualType::DK_nontrivial_c_struct) && "unexpected destructor type"); - pushDestroy(DtorKind, DeclPtr, Ty); - CalleeDestructedParamCleanups[cast<ParmVarDecl>(&D)] = - EHStack.stable_begin(); + // Don't push the destructor if this is the parameter of an ObjC + // synthesized setter and its type is a C struct that isn't trivially + // copyable. The setter moves the parameter instead of copying it. + auto *PVD = dyn_cast<ParmVarDecl>(&D); + if (!PVD || !PVD->isSynthesizedObjCPropertySetterParameter() || + Ty.isNonTrivialToPrimitiveCopy() != QualType::PCK_Struct) { + pushDestroy(DtorKind, DeclPtr, Ty); + CalleeDestructedParamCleanups[cast<ParmVarDecl>(&D)] = + EHStack.stable_begin(); + } } } } else { Index: clang/lib/AST/Decl.cpp =================================================================== --- clang/lib/AST/Decl.cpp +++ clang/lib/AST/Decl.cpp @@ -2848,6 +2848,12 @@ return DeclaratorDecl::getSourceRange(); } +bool ParmVarDecl::isSynthesizedObjCPropertySetterParameter() const { + auto *OMD = dyn_cast<ObjCMethodDecl>(getDeclContext()); + return OMD && OMD->isPropertyAccessor() && OMD->isImplicit() && + OMD->param_size(); +} + bool ParmVarDecl::isDestroyedInCallee() const { // ns_consumed only affects code generation in ARC if (hasAttr<NSConsumedAttr>()) Index: clang/include/clang/AST/Decl.h =================================================================== --- clang/include/clang/AST/Decl.h +++ clang/include/clang/AST/Decl.h @@ -1729,6 +1729,10 @@ return ParmVarDeclBits.IsObjCMethodParam; } + /// Determines whether this is the source parameter of a synthesized ObjC + /// property setter. + bool isSynthesizedObjCPropertySetterParameter() const; + /// Determines whether this parameter is destroyed in the callee function. bool isDestroyedInCallee() const;
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits