From 27ecc11915b5b53db707aca267775e5a734498e5 Mon Sep 17 00:00:00 2001 From: David Nadlinger Date: Thu, 26 Dec 2013 05:35:14 +0100 Subject: [PATCH 1/2] Remove unused prototype. --- gen/structs.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/gen/structs.cpp b/gen/structs.cpp index 690d93d7..c063b612 100644 --- a/gen/structs.cpp +++ b/gen/structs.cpp @@ -121,9 +121,6 @@ LLValue* DtoIndexStruct(LLValue* src, StructDeclaration* sd, VarDeclaration* vd) ////////////////////////////////////////////////////////////////////////////////////////// -// helper function that adds zero bytes to a vector of constants -extern size_t add_zeros(std::vector& values, size_t diff); - /// Return the type returned by DtoUnpaddedStruct called on a value of the /// specified type. /// Union types will get expanded into a struct, with a type for each member. From f85d2a5a0a19c5ea339d3f1a9b7b905b6a5d1d8f Mon Sep 17 00:00:00 2001 From: David Nadlinger Date: Thu, 26 Dec 2013 06:23:39 +0100 Subject: [PATCH 2/2] Respect type alignment when choosing padding fields. Fixes ABI mismatch when e.g. padding a 28 byte union from a 20 byte member, where previously, an i64 would be emitted, yielding a 32 byte LLVM struct size on ulong.alignof == 8 platforms. Test case will follow on the 2.064 branch. --- ir/iraggr.cpp | 54 +++++++++++++++++++++++++++------------------ ir/irclass.cpp | 1 - ir/irtypeclass.cpp | 9 ++++---- ir/irtypestruct.cpp | 48 ++++++++++++++++++++++++++-------------- 4 files changed, 70 insertions(+), 42 deletions(-) diff --git a/ir/iraggr.cpp b/ir/iraggr.cpp index 9cb8017e..1aca1afe 100644 --- a/ir/iraggr.cpp +++ b/ir/iraggr.cpp @@ -87,34 +87,50 @@ llvm::Constant * IrAggr::getDefaultInit() ////////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////// +static bool isAligned(llvm::Type* type, size_t offset) { + return gDataLayout->getABITypeAlignment(type) % offset == 0; +} + // helper function that adds zero bytes to a vector of constants -size_t add_zeros(llvm::SmallVectorImpl& constants, size_t diff) +size_t add_zeros(llvm::SmallVectorImpl& constants, + size_t startOffset, size_t endOffset) { - size_t n = constants.size(); - while (diff) + size_t const oldLength = constants.size(); + + llvm::Type* const eightByte = llvm::Type::getInt64Ty(gIR->context()); + llvm::Type* const fourByte = llvm::Type::getInt32Ty(gIR->context()); + llvm::Type* const twoByte = llvm::Type::getInt16Ty(gIR->context()); + + assert(startOffset <= endOffset); + size_t paddingLeft = endOffset - startOffset; + while (paddingLeft) { - if (global.params.is64bit && diff % 8 == 0) + if (global.params.is64bit && paddingLeft >= 8 && isAligned(eightByte, startOffset)) { - constants.push_back(LLConstant::getNullValue(llvm::Type::getInt64Ty(gIR->context()))); - diff -= 8; + constants.push_back(llvm::Constant::getNullValue(eightByte)); + startOffset += 8; } - else if (diff % 4 == 0) + else if (paddingLeft >= 4 && isAligned(fourByte, startOffset)) { - constants.push_back(LLConstant::getNullValue(llvm::Type::getInt32Ty(gIR->context()))); - diff -= 4; + constants.push_back(llvm::Constant::getNullValue(fourByte)); + startOffset += 4; } - else if (diff % 2 == 0) + else if (paddingLeft >= 2 && isAligned(twoByte, startOffset)) { - constants.push_back(LLConstant::getNullValue(llvm::Type::getInt16Ty(gIR->context()))); - diff -= 2; + constants.push_back(llvm::Constant::getNullValue(twoByte)); + startOffset += 2; } else { - constants.push_back(LLConstant::getNullValue(llvm::Type::getInt8Ty(gIR->context()))); - diff -= 1; + constants.push_back(llvm::Constant::getNullValue( + llvm::Type::getInt8Ty(gIR->context()))); + startOffset += 1; } + + paddingLeft = endOffset - startOffset; } - return constants.size() - n; + + return constants.size() - oldLength; } ////////////////////////////////////////////////////////////////////////////// @@ -263,9 +279,7 @@ llvm::Constant* IrAggr::createInitializerConstant( const size_t structsize = type->size(); if (offset < structsize) { - size_t diff = structsize - offset; - IF_LOG Logger::println("adding %zu bytes zero padding", diff); - add_zeros(constants, diff); + add_zeros(constants, offset, structsize); } // get initializer type @@ -386,9 +400,7 @@ void IrAggr::addFieldInitializers( // insert explicit padding? if (alignedoffset < vd->offset) { - size_t diff = vd->offset - alignedoffset; - IF_LOG Logger::println("adding %zu bytes zero padding", diff); - add_zeros(constants, diff); + add_zeros(constants, alignedoffset, vd->offset); } IF_LOG Logger::println("adding field %s", vd->toChars()); diff --git a/ir/irclass.cpp b/ir/irclass.cpp index 001623b9..e28dc049 100644 --- a/ir/irclass.cpp +++ b/ir/irclass.cpp @@ -37,7 +37,6 @@ ////////////////////////////////////////////////////////////////////////////// extern LLConstant* get_default_initializer(VarDeclaration* vd, Initializer* init); -extern size_t add_zeros(std::vector& constants, size_t diff); extern LLConstant* DtoDefineClassInfo(ClassDeclaration* cd); diff --git a/ir/irtypeclass.cpp b/ir/irtypeclass.cpp index de8c44be..3fff9f2a 100644 --- a/ir/irtypeclass.cpp +++ b/ir/irtypeclass.cpp @@ -30,7 +30,8 @@ ////////////////////////////////////////////////////////////////////////////// -extern size_t add_zeros(std::vector& defaultTypes, size_t diff); +extern size_t add_zeros(std::vector& defaultTypes, + size_t startOffset, size_t endOffset); extern bool var_offset_sort_cb(const VarDeclaration* v1, const VarDeclaration* v2); ////////////////////////////////////////////////////////////////////////////// @@ -167,7 +168,7 @@ void IrTypeClass::addBaseClassData( // insert explicit padding? if (alignedoffset < vd->offset) { - field_index += add_zeros(defaultTypes, vd->offset - alignedoffset); + field_index += add_zeros(defaultTypes, alignedoffset, vd->offset); } // add default type @@ -220,7 +221,7 @@ void IrTypeClass::addBaseClassData( // tail padding? if (offset < base->structsize) { - field_index += add_zeros(defaultTypes, base->structsize - offset); + field_index += add_zeros(defaultTypes, offset, base->structsize); offset = base->structsize; } #endif @@ -268,7 +269,7 @@ IrTypeClass* IrTypeClass::get(ClassDeclaration* cd) // tail padding? if (offset < cd->structsize) { - field_index += add_zeros(defaultTypes, cd->structsize - offset); + field_index += add_zeros(defaultTypes, offset, cd->structsize); offset = cd->structsize; } #endif diff --git a/ir/irtypestruct.cpp b/ir/irtypestruct.cpp index 44b805fa..291a1963 100644 --- a/ir/irtypestruct.cpp +++ b/ir/irtypestruct.cpp @@ -38,35 +38,51 @@ IrTypeStruct::IrTypeStruct(StructDeclaration * sd) ////////////////////////////////////////////////////////////////////////////// -size_t add_zeros(std::vector& defaultTypes, size_t diff) +static bool isAligned(llvm::Type* type, size_t offset) { + return gDataLayout->getABITypeAlignment(type) % offset == 0; +} + +size_t add_zeros(std::vector& defaultTypes, + size_t startOffset, size_t endOffset) { - size_t n = defaultTypes.size(); - while (diff) + size_t const oldLength = defaultTypes.size(); + + llvm::Type* const eightByte = llvm::Type::getInt64Ty(gIR->context()); + llvm::Type* const fourByte = llvm::Type::getInt32Ty(gIR->context()); + llvm::Type* const twoByte = llvm::Type::getInt16Ty(gIR->context()); + + assert(startOffset <= endOffset); + size_t paddingLeft = endOffset - startOffset; + while (paddingLeft) { - if (global.params.is64bit && diff % 8 == 0) + if (global.params.is64bit && paddingLeft >= 8 && isAligned(eightByte, startOffset)) { - defaultTypes.push_back(llvm::Type::getInt64Ty(gIR->context())); - diff -= 8; + defaultTypes.push_back(eightByte); + startOffset += 8; } - else if (diff % 4 == 0) + else if (paddingLeft >= 4 && isAligned(fourByte, startOffset)) { - defaultTypes.push_back(llvm::Type::getInt32Ty(gIR->context())); - diff -= 4; + defaultTypes.push_back(fourByte); + startOffset += 4; } - else if (diff % 2 == 0) + else if (paddingLeft >= 2 && isAligned(twoByte, startOffset)) { - defaultTypes.push_back(llvm::Type::getInt16Ty(gIR->context())); - diff -= 2; + defaultTypes.push_back(twoByte); + startOffset += 2; } else { defaultTypes.push_back(llvm::Type::getInt8Ty(gIR->context())); - diff -= 1; + startOffset += 1; } + + paddingLeft = endOffset - startOffset; } - return defaultTypes.size() - n; + + return defaultTypes.size() - oldLength; } + bool var_offset_sort_cb(const VarDeclaration* v1, const VarDeclaration* v2) { if (v1 && v2) @@ -209,7 +225,7 @@ IrTypeStruct* IrTypeStruct::get(StructDeclaration* sd) // insert explicit padding? if (alignedoffset < vd->offset) { - field_index += add_zeros(defaultTypes, vd->offset - alignedoffset); + field_index += add_zeros(defaultTypes, alignedoffset, vd->offset); } // add default type @@ -225,7 +241,7 @@ IrTypeStruct* IrTypeStruct::get(StructDeclaration* sd) // tail padding? if (offset < sd->structsize) { - add_zeros(defaultTypes, sd->structsize - offset); + add_zeros(defaultTypes, offset, sd->structsize); } // set struct body