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> 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 > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
