r342053 - [CodeGen] Align rtti and vtable data

Eric Christopher via cfe-commits cfe-commits at lists.llvm.org
Mon Sep 17 10:07:47 PDT 2018


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 at 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 at Test1@@7B01@@" = linkonce_odr unnamed_addr
> constant [4 x i32] [i32 0, i32 8, i32 12, i32 20]
> +// CHECK-DAG: @"??_8D at Test1@@7B01@@" = linkonce_odr unnamed_addr
> constant [4 x i32] [i32 0, i32 8, i32 12, i32 20], comdat, align 4
>  // CHECK-DAG: @"??_8D at Test1@@7BB at 1@@" = {{.*}} [2 x i32] [i32 0, i32 -4]
>  // CHECK-DAG: @"??_8D at Test1@@7BC at 1@@" = {{.*}} [2 x i32] [i32 0, i32 -12]
>  // CHECK-DAG: @"??_8C at 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 at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180917/db0c0cf9/attachment-0001.html>


More information about the cfe-commits mailing list