Hello
Interesting, what kind of failures? If they are causing you problems, of course feel free to revert. Dave From: Eric Christopher <echri...@gmail.com> Sent: 17 September 2018 18:07:47 To: David Green Cc: cfe-commits@lists.llvm.org Subject: Re: r342053 - [CodeGen] Align rtti and vtable data Hi David, I'm seeing test failures after this patch. I'm trying to get a test case reduced, but can we revert until we figure it out? Thanks! -eric On Wed, Sep 12, 2018 at 7:10 AM David Green via cfe-commits <cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>> wrote: Author: dmgreen Date: Wed Sep 12 07:09:06 2018 New Revision: 342053 URL: http://llvm.org/viewvc/llvm-project?rev=342053&view=rev Log: [CodeGen] Align rtti and vtable data Previously the alignment on the newly created rtti/typeinfo data was largely not set, meaning that DataLayout::getPreferredAlignment was free to overalign it to 16 bytes. This causes unnecessary code bloat. Differential Revision: https://reviews.llvm.org/D51416 Modified: cfe/trunk/lib/CodeGen/CGVTT.cpp cfe/trunk/lib/CodeGen/CGVTables.cpp cfe/trunk/lib/CodeGen/CodeGenModule.cpp cfe/trunk/lib/CodeGen/CodeGenModule.h cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp cfe/trunk/test/CodeGenCXX/microsoft-abi-vbtables.cpp cfe/trunk/test/CodeGenCXX/vtable-align.cpp cfe/trunk/test/CodeGenCXX/vtable-linkage.cpp Modified: cfe/trunk/lib/CodeGen/CGVTT.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGVTT.cpp?rev=342053&r1=342052&r2=342053&view=diff ============================================================================== --- cfe/trunk/lib/CodeGen/CGVTT.cpp (original) +++ cfe/trunk/lib/CodeGen/CGVTT.cpp Wed Sep 12 07:09:06 2018 @@ -119,10 +119,10 @@ llvm::GlobalVariable *CodeGenVTables::Ge llvm::ArrayType *ArrayType = llvm::ArrayType::get(CGM.Int8PtrTy, Builder.getVTTComponents().size()); + unsigned Align = CGM.getDataLayout().getABITypeAlignment(CGM.Int8PtrTy); - llvm::GlobalVariable *GV = - CGM.CreateOrReplaceCXXRuntimeVariable(Name, ArrayType, - llvm::GlobalValue::ExternalLinkage); + llvm::GlobalVariable *GV = CGM.CreateOrReplaceCXXRuntimeVariable( + Name, ArrayType, llvm::GlobalValue::ExternalLinkage, Align); GV->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global); return GV; } Modified: cfe/trunk/lib/CodeGen/CGVTables.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGVTables.cpp?rev=342053&r1=342052&r2=342053&view=diff ============================================================================== --- cfe/trunk/lib/CodeGen/CGVTables.cpp (original) +++ cfe/trunk/lib/CodeGen/CGVTables.cpp Wed Sep 12 07:09:06 2018 @@ -756,9 +756,11 @@ CodeGenVTables::GenerateConstructionVTab if (Linkage == llvm::GlobalVariable::AvailableExternallyLinkage) Linkage = llvm::GlobalVariable::InternalLinkage; + unsigned Align = CGM.getDataLayout().getABITypeAlignment(VTType); + // Create the variable that will hold the construction vtable. llvm::GlobalVariable *VTable = - CGM.CreateOrReplaceCXXRuntimeVariable(Name, VTType, Linkage); + CGM.CreateOrReplaceCXXRuntimeVariable(Name, VTType, Linkage, Align); CGM.setGVProperties(VTable, RD); // V-tables are always unnamed_addr. Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=342053&r1=342052&r2=342053&view=diff ============================================================================== --- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original) +++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Wed Sep 12 07:09:06 2018 @@ -3099,10 +3099,9 @@ CodeGenModule::GetAddrOfGlobal(GlobalDec IsForDefinition); } -llvm::GlobalVariable * -CodeGenModule::CreateOrReplaceCXXRuntimeVariable(StringRef Name, - llvm::Type *Ty, - llvm::GlobalValue::LinkageTypes Linkage) { +llvm::GlobalVariable *CodeGenModule::CreateOrReplaceCXXRuntimeVariable( + StringRef Name, llvm::Type *Ty, llvm::GlobalValue::LinkageTypes Linkage, + unsigned Alignment) { llvm::GlobalVariable *GV = getModule().getNamedGlobal(Name); llvm::GlobalVariable *OldGV = nullptr; @@ -3138,6 +3137,8 @@ CodeGenModule::CreateOrReplaceCXXRuntime !GV->hasAvailableExternallyLinkage()) GV->setComdat(TheModule.getOrInsertComdat(GV->getName())); + GV->setAlignment(Alignment); + return GV; } Modified: cfe/trunk/lib/CodeGen/CodeGenModule.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.h?rev=342053&r1=342052&r2=342053&view=diff ============================================================================== --- cfe/trunk/lib/CodeGen/CodeGenModule.h (original) +++ cfe/trunk/lib/CodeGen/CodeGenModule.h Wed Sep 12 07:09:06 2018 @@ -764,7 +764,8 @@ public: /// bitcast to the new variable. llvm::GlobalVariable * CreateOrReplaceCXXRuntimeVariable(StringRef Name, llvm::Type *Ty, - llvm::GlobalValue::LinkageTypes Linkage); + llvm::GlobalValue::LinkageTypes Linkage, + unsigned Alignment); llvm::Function * CreateGlobalInitOrDestructFunction(llvm::FunctionType *ty, const Twine &name, Modified: cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp?rev=342053&r1=342052&r2=342053&view=diff ============================================================================== --- cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp (original) +++ cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp Wed Sep 12 07:09:06 2018 @@ -1598,12 +1598,6 @@ void ItaniumCXXABI::emitVTableDefinition // Set the right visibility. CGM.setGVProperties(VTable, RD); - // Use pointer alignment for the vtable. Otherwise we would align them based - // on the size of the initializer which doesn't make sense as only single - // values are read. - unsigned PAlign = CGM.getTarget().getPointerAlign(0); - VTable->setAlignment(getContext().toCharUnitsFromBits(PAlign).getQuantity()); - // If this is the magic class __cxxabiv1::__fundamental_type_info, // we will emit the typeinfo for the fundamental types. This is the // same behaviour as GCC. @@ -1703,8 +1697,14 @@ llvm::GlobalVariable *ItaniumCXXABI::get CGM.getItaniumVTableContext().getVTableLayout(RD); llvm::Type *VTableType = CGM.getVTables().getVTableType(VTLayout); + // Use pointer alignment for the vtable. Otherwise we would align them based + // on the size of the initializer which doesn't make sense as only single + // values are read. + unsigned PAlign = CGM.getTarget().getPointerAlign(0); + VTable = CGM.CreateOrReplaceCXXRuntimeVariable( - Name, VTableType, llvm::GlobalValue::ExternalLinkage); + Name, VTableType, llvm::GlobalValue::ExternalLinkage, + getContext().toCharUnitsFromBits(PAlign).getQuantity()); VTable->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global); CGM.setGVProperties(VTable, RD); @@ -2725,9 +2725,10 @@ llvm::GlobalVariable *ItaniumRTTIBuilder // get the mangled name of the type. llvm::Constant *Init = llvm::ConstantDataArray::getString(VMContext, Name.substr(4)); + auto Align = CGM.getContext().getTypeAlignInChars(CGM.getContext().CharTy); - llvm::GlobalVariable *GV = - CGM.CreateOrReplaceCXXRuntimeVariable(Name, Init->getType(), Linkage); + llvm::GlobalVariable *GV = CGM.CreateOrReplaceCXXRuntimeVariable( + Name, Init->getType(), Linkage, Align.getQuantity()); GV->setInitializer(Init); @@ -3366,6 +3367,10 @@ llvm::Constant *ItaniumRTTIBuilder::Buil if (CGM.supportsCOMDAT() && GV->isWeakForLinker()) GV->setComdat(M.getOrInsertComdat(GV->getName())); + CharUnits Align = + CGM.getContext().toCharUnitsFromBits(CGM.getTarget().getPointerAlign(0)); + GV->setAlignment(Align.getQuantity()); + // The Itanium ABI specifies that type_info objects must be globally // unique, with one exception: if the type is an incomplete class // type or a (possibly indirect) pointer to one. That exception Modified: cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp?rev=342053&r1=342052&r2=342053&view=diff ============================================================================== --- cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp (original) +++ cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp Wed Sep 12 07:09:06 2018 @@ -2024,8 +2024,10 @@ MicrosoftCXXABI::getAddrOfVBTable(const assert(!CGM.getModule().getNamedGlobal(Name) && "vbtable with this name already exists: mangling bug?"); - llvm::GlobalVariable *GV = - CGM.CreateOrReplaceCXXRuntimeVariable(Name, VBTableType, Linkage); + CharUnits Alignment = + CGM.getContext().getTypeAlignInChars(CGM.getContext().IntTy); + llvm::GlobalVariable *GV = CGM.CreateOrReplaceCXXRuntimeVariable( + Name, VBTableType, Linkage, Alignment.getQuantity()); GV->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global); if (RD->hasAttr<DLLImportAttr>()) Modified: cfe/trunk/test/CodeGenCXX/microsoft-abi-vbtables.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/microsoft-abi-vbtables.cpp?rev=342053&r1=342052&r2=342053&view=diff ============================================================================== --- cfe/trunk/test/CodeGenCXX/microsoft-abi-vbtables.cpp (original) +++ cfe/trunk/test/CodeGenCXX/microsoft-abi-vbtables.cpp Wed Sep 12 07:09:06 2018 @@ -19,7 +19,7 @@ D d; // Force vbtable emission. // C: vbptr C // int c -// CHECK-DAG: @"??_8D@Test1@@7B01@@" = linkonce_odr unnamed_addr constant [4 x i32] [i32 0, i32 8, i32 12, i32 20] +// CHECK-DAG: @"??_8D@Test1@@7B01@@" = linkonce_odr unnamed_addr constant [4 x i32] [i32 0, i32 8, i32 12, i32 20], comdat, align 4 // CHECK-DAG: @"??_8D@Test1@@7BB@1@@" = {{.*}} [2 x i32] [i32 0, i32 -4] // CHECK-DAG: @"??_8D@Test1@@7BC@1@@" = {{.*}} [2 x i32] [i32 0, i32 -12] // CHECK-DAG: @"??_8C@Test1@@7B@" = {{.*}} [2 x i32] [i32 0, i32 8] Modified: cfe/trunk/test/CodeGenCXX/vtable-align.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/vtable-align.cpp?rev=342053&r1=342052&r2=342053&view=diff ============================================================================== --- cfe/trunk/test/CodeGenCXX/vtable-align.cpp (original) +++ cfe/trunk/test/CodeGenCXX/vtable-align.cpp Wed Sep 12 07:09:06 2018 @@ -10,5 +10,8 @@ struct A { void A::f() {} // CHECK-32: @_ZTV1A = unnamed_addr constant { [5 x i8*] } { [5 x i8*] [i8* null, i8* bitcast ({ i8*, i8* }* @_ZTI1A to i8*), i8* bitcast (void (%struct.A*)* @_ZN1A1fEv to i8*), i8* bitcast (void (%struct.A*)* @_ZN1A1gEv to i8*), i8* bitcast (void (%struct.A*)* @_ZN1A1hEv to i8*)] }, align 4 - +// CHECK-32: @_ZTS1A = constant [3 x i8] c"1A\00", align 1 +// CHECK-32: @_ZTI1A = constant { i8*, i8* } { i8* bitcast (i8** getelementptr inbounds (i8*, i8** @_ZTVN10__cxxabiv117__class_type_infoE, i32 2) to i8*), i8* getelementptr inbounds ([3 x i8], [3 x i8]* @_ZTS1A, i32 0, i32 0) }, align 4 // CHECK-64: @_ZTV1A = unnamed_addr constant { [5 x i8*] } { [5 x i8*] [i8* null, i8* bitcast ({ i8*, i8* }* @_ZTI1A to i8*), i8* bitcast (void (%struct.A*)* @_ZN1A1fEv to i8*), i8* bitcast (void (%struct.A*)* @_ZN1A1gEv to i8*), i8* bitcast (void (%struct.A*)* @_ZN1A1hEv to i8*)] }, align 8 +// CHECK-64: @_ZTS1A = constant [3 x i8] c"1A\00", align 1 +// CHECK-64: @_ZTI1A = constant { i8*, i8* } { i8* bitcast (i8** getelementptr inbounds (i8*, i8** @_ZTVN10__cxxabiv117__class_type_infoE, i64 2) to i8*), i8* getelementptr inbounds ([3 x i8], [3 x i8]* @_ZTS1A, i32 0, i32 0) }, align 8 Modified: cfe/trunk/test/CodeGenCXX/vtable-linkage.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/vtable-linkage.cpp?rev=342053&r1=342052&r2=342053&view=diff ============================================================================== --- cfe/trunk/test/CodeGenCXX/vtable-linkage.cpp (original) +++ cfe/trunk/test/CodeGenCXX/vtable-linkage.cpp Wed Sep 12 07:09:06 2018 @@ -98,10 +98,10 @@ void use_F() { // C has no key function, so its vtable should have weak_odr linkage // and hidden visibility (rdar://problem/7523229). -// CHECK-DAG: @_ZTV1C = linkonce_odr unnamed_addr constant {{.*}}, comdat, -// CHECK-DAG: @_ZTS1C = linkonce_odr constant {{.*}}, comdat{{$}} -// CHECK-DAG: @_ZTI1C = linkonce_odr constant {{.*}}, comdat{{$}} -// CHECK-DAG: @_ZTT1C = linkonce_odr unnamed_addr constant {{.*}}, comdat{{$}} +// CHECK-DAG: @_ZTV1C = linkonce_odr unnamed_addr constant {{.*}}, comdat, align 8{{$}} +// CHECK-DAG: @_ZTS1C = linkonce_odr constant {{.*}}, comdat, align 1{{$}} +// CHECK-DAG: @_ZTI1C = linkonce_odr constant {{.*}}, comdat, align 8{{$}} +// CHECK-DAG: @_ZTT1C = linkonce_odr unnamed_addr constant {{.*}}, comdat, align 8{{$}} // D has a key function that is defined in this translation unit so its vtable is // defined in the translation unit. @@ -120,27 +120,27 @@ void use_F() { // defined in this translation unit, so its vtable should have // weak_odr linkage. // CHECK-DAG: @_ZTV1EIsE = weak_odr unnamed_addr constant {{.*}}, comdat, -// CHECK-DAG: @_ZTS1EIsE = weak_odr constant {{.*}}, comdat{{$}} -// CHECK-DAG: @_ZTI1EIsE = weak_odr constant {{.*}}, comdat{{$}} +// CHECK-DAG: @_ZTS1EIsE = weak_odr constant {{.*}}, comdat, align 1{{$}} +// CHECK-DAG: @_ZTI1EIsE = weak_odr constant {{.*}}, comdat, align 8{{$}} // F<short> is an explicit template instantiation without a key // function, so its vtable should have weak_odr linkage // CHECK-DAG: @_ZTV1FIsE = weak_odr unnamed_addr constant {{.*}}, comdat, -// CHECK-DAG: @_ZTS1FIsE = weak_odr constant {{.*}}, comdat{{$}} -// CHECK-DAG: @_ZTI1FIsE = weak_odr constant {{.*}}, comdat{{$}} +// CHECK-DAG: @_ZTS1FIsE = weak_odr constant {{.*}}, comdat, align 1{{$}} +// CHECK-DAG: @_ZTI1FIsE = weak_odr constant {{.*}}, comdat, align 8{{$}} // E<long> is an implicit template instantiation with a key function // defined in this translation unit, so its vtable should have // linkonce_odr linkage. // CHECK-DAG: @_ZTV1EIlE = linkonce_odr unnamed_addr constant {{.*}}, comdat, -// CHECK-DAG: @_ZTS1EIlE = linkonce_odr constant {{.*}}, comdat{{$}} -// CHECK-DAG: @_ZTI1EIlE = linkonce_odr constant {{.*}}, comdat{{$}} +// CHECK-DAG: @_ZTS1EIlE = linkonce_odr constant {{.*}}, comdat, align 1{{$}} +// CHECK-DAG: @_ZTI1EIlE = linkonce_odr constant {{.*}}, comdat, align 8{{$}} // F<long> is an implicit template instantiation with no key function, // so its vtable should have linkonce_odr linkage. // CHECK-DAG: @_ZTV1FIlE = linkonce_odr unnamed_addr constant {{.*}}, comdat, -// CHECK-DAG: @_ZTS1FIlE = linkonce_odr constant {{.*}}, comdat{{$}} -// CHECK-DAG: @_ZTI1FIlE = linkonce_odr constant {{.*}}, comdat{{$}} +// CHECK-DAG: @_ZTS1FIlE = linkonce_odr constant {{.*}}, comdat, align 1{{$}} +// CHECK-DAG: @_ZTI1FIlE = linkonce_odr constant {{.*}}, comdat, align 8{{$}} // F<int> is an explicit template instantiation declaration without a // key function, so its vtable should have external linkage. @@ -171,8 +171,8 @@ void use_F() { // F<char> is an explicit specialization without a key function, so // its vtable should have linkonce_odr linkage. // CHECK-DAG: @_ZTV1FIcE = linkonce_odr unnamed_addr constant {{.*}}, comdat, -// CHECK-DAG: @_ZTS1FIcE = linkonce_odr constant {{.*}}, comdat{{$}} -// CHECK-DAG: @_ZTI1FIcE = linkonce_odr constant {{.*}}, comdat{{$}} +// CHECK-DAG: @_ZTS1FIcE = linkonce_odr constant {{.*}}, comdat, align 1{{$}} +// CHECK-DAG: @_ZTI1FIcE = linkonce_odr constant {{.*}}, comdat, align 8{{$}} // CHECK-DAG: @_ZTV1GIiE = linkonce_odr unnamed_addr constant {{.*}}, comdat, template <typename T> _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
