This regressed v8 garbage collection performance by 10%. Not sure if the optimizations you're alluding to would help. I filed PR14206 for this.
On Mon, Aug 20, 2012 at 9:10 PM, John McCall <[email protected]> wrote: > Author: rjmccall > Date: Mon Aug 20 23:10:00 2012 > New Revision: 162254 > > URL: http://llvm.org/viewvc/llvm-project?rev=162254&view=rev > Log: > When performing a trivial copy of a C++ type, we must be careful not > to overwrite objects that might have been allocated into the type's > tail padding. This patch is missing some potential optimizations where > the destination is provably a complete object, but it's necessary for > correctness. > > Patch by Jonathan Sauer. > > Modified: > cfe/trunk/include/clang/AST/ASTContext.h > cfe/trunk/lib/AST/ASTContext.cpp > cfe/trunk/lib/CodeGen/CGExprAgg.cpp > cfe/trunk/test/CodeGenObjCXX/implicit-copy-assign-operator.mm > > Modified: cfe/trunk/include/clang/AST/ASTContext.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=162254&r1=162253&r2=162254&view=diff > ============================================================================== > --- cfe/trunk/include/clang/AST/ASTContext.h (original) > +++ cfe/trunk/include/clang/AST/ASTContext.h Mon Aug 20 23:10:00 2012 > @@ -1427,6 +1427,10 @@ > /// characters. This method does not work on incomplete types. > CharUnits getTypeAlignInChars(QualType T) const; > CharUnits getTypeAlignInChars(const Type *T) const; > + > + // getTypeInfoDataSizeInChars - Return the size of a type, in chars. If the > + // type is a record, its data size is returned. > + std::pair<CharUnits, CharUnits> getTypeInfoDataSizeInChars(QualType T) > const; > > std::pair<CharUnits, CharUnits> getTypeInfoInChars(const Type *T) const; > std::pair<CharUnits, CharUnits> getTypeInfoInChars(QualType T) const; > > Modified: cfe/trunk/lib/AST/ASTContext.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=162254&r1=162253&r2=162254&view=diff > ============================================================================== > --- cfe/trunk/lib/AST/ASTContext.cpp (original) > +++ cfe/trunk/lib/AST/ASTContext.cpp Mon Aug 20 23:10:00 2012 > @@ -1080,6 +1080,27 @@ > return toCharUnitsFromBits(Align); > } > > +// getTypeInfoDataSizeInChars - Return the size of a type, in > +// chars. If the type is a record, its data size is returned. This is > +// the size of the memcpy that's performed when assigning this type > +// using a trivial copy/move assignment operator. > +std::pair<CharUnits, CharUnits> > +ASTContext::getTypeInfoDataSizeInChars(QualType T) const { > + std::pair<CharUnits, CharUnits> sizeAndAlign = getTypeInfoInChars(T); > + > + // In C++, objects can sometimes be allocated into the tail padding > + // of a base-class subobject. We decide whether that's possible > + // during class layout, so here we can just trust the layout results. > + if (getLangOpts().CPlusPlus) { > + if (const RecordType *RT = T->getAs<RecordType>()) { > + const ASTRecordLayout &layout = getASTRecordLayout(RT->getDecl()); > + sizeAndAlign.first = layout.getDataSize(); > + } > + } > + > + return sizeAndAlign; > +} > + > std::pair<CharUnits, CharUnits> > ASTContext::getTypeInfoInChars(const Type *T) const { > std::pair<uint64_t, unsigned> Info = getTypeInfo(T); > > Modified: cfe/trunk/lib/CodeGen/CGExprAgg.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprAgg.cpp?rev=162254&r1=162253&r2=162254&view=diff > ============================================================================== > --- cfe/trunk/lib/CodeGen/CGExprAgg.cpp (original) > +++ cfe/trunk/lib/CodeGen/CGExprAgg.cpp Mon Aug 20 23:10:00 2012 > @@ -1300,9 +1300,9 @@ > // implementation handles this case safely. If there is a libc that does > not > // safely handle this, we can add a target hook. > > - // Get size and alignment info for this aggregate. > + // Get data size and alignment info for this aggregate. > std::pair<CharUnits, CharUnits> TypeInfo = > - getContext().getTypeInfoInChars(Ty); > + getContext().getTypeInfoDataSizeInChars(Ty); > > if (alignment.isZero()) > alignment = TypeInfo.second; > > Modified: cfe/trunk/test/CodeGenObjCXX/implicit-copy-assign-operator.mm > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjCXX/implicit-copy-assign-operator.mm?rev=162254&r1=162253&r2=162254&view=diff > ============================================================================== > --- cfe/trunk/test/CodeGenObjCXX/implicit-copy-assign-operator.mm (original) > +++ cfe/trunk/test/CodeGenObjCXX/implicit-copy-assign-operator.mm Mon Aug 20 > 23:10:00 2012 > @@ -1,4 +1,6 @@ > -// RUN: %clang_cc1 -fobjc-gc -emit-llvm -triple x86_64-apple-darwin10.0.0 > -fobjc-runtime=macosx-fragile-10.5 -o - %s | FileCheck %s > +// RUN: %clang_cc1 -fobjc-gc -emit-llvm -triple x86_64-apple-darwin10.0.0 > -fobjc-runtime=macosx-fragile-10.5 -o - %s | FileCheck %s > -check-prefix=CHECK-OBJ > +// RUN: %clang_cc1 -x c++ -emit-llvm -triple x86_64-apple-darwin10.0.0 > -o - %s | FileCheck %s -check-prefix=CHECK-CPP > +#ifdef __OBJC__ > struct A { > A &operator=(const A&); > A &operator=(A&); > @@ -41,17 +43,82 @@ > d1 = d2; > } > > -// CHECK: define linkonce_odr %struct.D* @_ZN1DaSERS_ > -// CHECK: {{call.*_ZN1AaSERS_}} > -// CHECK: {{call.*_ZN1BaSERS_}} > -// CHECK: {{call.*_ZN1CaSERKS_}} > -// CHECK: {{call void @llvm.memcpy.p0i8.p0i8.i64.*i64 24}} > -// CHECK: {{call.*_ZN1BaSERS_}} > -// CHECK: br > -// CHECK: {{call.*_ZN1CaSERKS_}} > -// CHECK: {{call.*@objc_memmove_collectable}} > -// CHECK: {{call void @llvm.memcpy.p0i8.p0i8.i64.*i64 12}} > -// CHECK: call void @_ZN11CopyByValueC1ERKS_ > -// CHECK: {{call.*_ZN11CopyByValueaSES_}} > -// CHECK: ret > - > +// CHECK-OBJ: define linkonce_odr %struct.D* @_ZN1DaSERS_ > +// CHECK-OBJ: {{call.*_ZN1AaSERS_}} > +// CHECK-OBJ: {{call.*_ZN1BaSERS_}} > +// CHECK-OBJ: {{call.*_ZN1CaSERKS_}} > +// CHECK-OBJ: {{call void @llvm.memcpy.p0i8.p0i8.i64.*i64 24}} > +// CHECK-OBJ: {{call.*_ZN1BaSERS_}} > +// CHECK-OBJ: br > +// CHECK-OBJ: {{call.*_ZN1CaSERKS_}} > +// CHECK-OBJ: {{call.*@objc_memmove_collectable}} > +// CHECK-OBJ: {{call void @llvm.memcpy.p0i8.p0i8.i64.*i64 12}} > +// CHECK-OBJ: call void @_ZN11CopyByValueC1ERKS_ > +// CHECK-OBJ: {{call.*_ZN11CopyByValueaSES_}} > +// CHECK-OBJ: ret > +#endif > + > +namespace PR13329 { > +#ifndef __OBJC__ > + typedef void* id; > +#endif > + struct POD { > + id i; > + short s; > + }; > + > + struct NonPOD { > + id i; > + short s; > + > + NonPOD(); > + }; > + > + struct DerivedNonPOD: NonPOD { > + char c; > + }; > + > + struct DerivedPOD: POD { > + char c; > + }; > + > + void testPOD() { > + POD a; > + POD b; > + // CHECK-OBJ: @objc_memmove_collectable{{.*}}i64 16 > + // CHECK-CPP: @llvm.memcpy{{.*}}i64 16 > + b = a; > + } > + > + void testNonPOD() { > + NonPOD a; > + NonPOD b; > + // CHECK-OBJ: @objc_memmove_collectable{{.*}}i64 10 > + // CHECK-CPP: @llvm.memcpy{{.*}}i64 10 > + b = a; > + } > + > + void testDerivedNonPOD() { > + DerivedNonPOD a; > + NonPOD b; > + DerivedNonPOD c; > + // CHECK-OBJ: @objc_memmove_collectable{{.*}}i64 10 > + // CHECK-CPP: @llvm.memcpy{{.*}}i64 10 > + (NonPOD&) a = b; > + // CHECK-OBJ: @objc_memmove_collectable{{.*}}i64 11 > + // CHECK-CPP: @llvm.memcpy{{.*}}i64 11 > + a = c; > + }; > + > + void testDerivedPOD() { > + DerivedPOD a; > + POD b; > + DerivedPOD c; > + // CHECK-OBJ: @objc_memmove_collectable{{.*}}i64 16 > + // CHECK-CPP: @llvm.memcpy{{.*}}i64 16 > + (POD&) a = b; > + // CHECK-OBJ: @objc_memmove_collectable{{.*}}i64 17 > + // CHECK-CPP: @llvm.memcpy{{.*}}i64 17 > + a = c; > + }; > +} > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
