dmgreen updated this revision to Diff 163727.
dmgreen retitled this revision from "[RTTI] Align rtti type string to prevent
over-alignment" to "[RTTI] Align rtti types to prevent over-alignment".
dmgreen edited the summary of this revision.
dmgreen added a comment.
I've become less sure about what the various alignments should be here. There
seem to be multiple ways to calculate such things, I'm not sure which are best.
For example, for want of a better option I've used getPointerAlign in
ItaniumRTTIBuilder::BuildTypeInfo.
https://reviews.llvm.org/D51416
Files:
lib/CodeGen/CGVTT.cpp
lib/CodeGen/CGVTables.cpp
lib/CodeGen/CodeGenModule.cpp
lib/CodeGen/CodeGenModule.h
lib/CodeGen/ItaniumCXXABI.cpp
lib/CodeGen/MicrosoftCXXABI.cpp
test/CodeGenCXX/vtable-align.cpp
test/CodeGenCXX/vtable-linkage.cpp
Index: test/CodeGenCXX/vtable-linkage.cpp
===================================================================
--- test/CodeGenCXX/vtable-linkage.cpp
+++ test/CodeGenCXX/vtable-linkage.cpp
@@ -98,10 +98,10 @@
// 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 @@
// 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 @@
// 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>
Index: test/CodeGenCXX/vtable-align.cpp
===================================================================
--- test/CodeGenCXX/vtable-align.cpp
+++ test/CodeGenCXX/vtable-align.cpp
@@ -10,5 +10,8 @@
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
Index: lib/CodeGen/MicrosoftCXXABI.cpp
===================================================================
--- lib/CodeGen/MicrosoftCXXABI.cpp
+++ lib/CodeGen/MicrosoftCXXABI.cpp
@@ -2024,8 +2024,9 @@
assert(!CGM.getModule().getNamedGlobal(Name) &&
"vbtable with this name already exists: mangling bug?");
+ unsigned Align = CGM.getClassPointerAlignment(RD).getQuantity();
llvm::GlobalVariable *GV =
- CGM.CreateOrReplaceCXXRuntimeVariable(Name, VBTableType, Linkage);
+ CGM.CreateOrReplaceCXXRuntimeVariable(Name, VBTableType, Linkage, Align);
GV->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
if (RD->hasAttr<DLLImportAttr>())
Index: lib/CodeGen/ItaniumCXXABI.cpp
===================================================================
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -1598,12 +1598,6 @@
// 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 @@
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);
@@ -2727,7 +2727,7 @@
Name.substr(4));
llvm::GlobalVariable *GV =
- CGM.CreateOrReplaceCXXRuntimeVariable(Name, Init->getType(), Linkage);
+ CGM.CreateOrReplaceCXXRuntimeVariable(Name, Init->getType(), Linkage, 1);
GV->setInitializer(Init);
@@ -3366,6 +3366,10 @@
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
Index: lib/CodeGen/CodeGenModule.h
===================================================================
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -764,7 +764,8 @@
/// 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,
Index: lib/CodeGen/CodeGenModule.cpp
===================================================================
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -3099,10 +3099,9 @@
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 @@
!GV->hasAvailableExternallyLinkage())
GV->setComdat(TheModule.getOrInsertComdat(GV->getName()));
+ GV->setAlignment(Alignment);
+
return GV;
}
Index: lib/CodeGen/CGVTables.cpp
===================================================================
--- lib/CodeGen/CGVTables.cpp
+++ lib/CodeGen/CGVTables.cpp
@@ -756,9 +756,11 @@
if (Linkage == llvm::GlobalVariable::AvailableExternallyLinkage)
Linkage = llvm::GlobalVariable::InternalLinkage;
+ unsigned Align = CGM.getClassPointerAlignment(RD).getQuantity();
+
// 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.
Index: lib/CodeGen/CGVTT.cpp
===================================================================
--- lib/CodeGen/CGVTT.cpp
+++ lib/CodeGen/CGVTT.cpp
@@ -119,10 +119,10 @@
llvm::ArrayType *ArrayType =
llvm::ArrayType::get(CGM.Int8PtrTy, Builder.getVTTComponents().size());
+ unsigned Align = CGM.getClassPointerAlignment(RD).getQuantity();
- 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;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
