r342053 - [CodeGen] Align rtti and vtable data

David Green via cfe-commits cfe-commits at lists.llvm.org
Mon Sep 17 10:19:33 PDT 2018


Hello


Interesting, what kind of failures?


If they are causing you problems, of course feel free to revert.

Dave


From: Eric Christopher <echristo at gmail.com>
Sent: 17 September 2018 18:07:47
To: David Green
Cc: cfe-commits at 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 at lists.llvm.org<mailto: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<mailto:cfe-commits at 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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180917/b69cbe45/attachment-0001.html>


More information about the cfe-commits mailing list