From 27ecc11915b5b53db707aca267775e5a734498e5 Mon Sep 17 00:00:00 2001 From: David Nadlinger Date: Thu, 26 Dec 2013 05:35:14 +0100 Subject: [PATCH 1/7] 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/7] 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 From bcbb13318a560dbff452bcca2fb9919687b02f2a Mon Sep 17 00:00:00 2001 From: David Nadlinger Date: Thu, 26 Dec 2013 12:39:52 +0100 Subject: [PATCH 3/7] Added test for GitHub #556. --- tests/d2/dmd-testsuite | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/d2/dmd-testsuite b/tests/d2/dmd-testsuite index 2834b349..fa4a570d 160000 --- a/tests/d2/dmd-testsuite +++ b/tests/d2/dmd-testsuite @@ -1 +1 @@ -Subproject commit 2834b34907483afc81f36e29e7963b490ba73813 +Subproject commit fa4a570db2c4257a0c0cec0a3ea96e82af244599 From be185263baf841d97a8eb440018e6c93a7db9f40 Mon Sep 17 00:00:00 2001 From: David Nadlinger Date: Thu, 26 Dec 2013 13:24:41 +0100 Subject: [PATCH 4/7] Disable internalizing of nested functions. This works around linking problems such as rejectedsoftware/vibe.d#338, caused by the frontend appending template instances to the wrong module. GitHub: Fixes #558. --- gen/tollvm.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/gen/tollvm.cpp b/gen/tollvm.cpp index 10241e71..c3a7d31b 100644 --- a/gen/tollvm.cpp +++ b/gen/tollvm.cpp @@ -382,6 +382,10 @@ LLGlobalValue::LinkageTypes DtoLinkage(Dsymbol* sym) llvm_unreachable("not global/function"); } + // The logic here should be sound in theory, but as long as the frontend + // keeps inserting templates into wrong modules, this yields to linking + // errors (see e.g. GitHub issue #558). +#if 0 // Check if sym is a nested function and we can declare it as internal. // // Nested naked functions and the implicitly generated __require/__ensure @@ -427,7 +431,7 @@ LLGlobalValue::LinkageTypes DtoLinkage(Dsymbol* sym) } } } - +#endif // default to external linkage return llvm::GlobalValue::ExternalLinkage; } From 333d538a23d17f1a58b8e3d9d753f3248cb587be Mon Sep 17 00:00:00 2001 From: David Nadlinger Date: Thu, 26 Dec 2013 12:42:31 +0100 Subject: [PATCH 5/7] Handle differing LLVM types for AA literals. This unsfortunately more or less duplicates the code we have for emitting ArrayLiteralExps, but with the different iteration strategies, having a single implementation would wind up even messier. Unfortunately, no regression test case yet, as I found this deep inside vibe.d. --- gen/toir.cpp | 42 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 36 insertions(+), 6 deletions(-) diff --git a/gen/toir.cpp b/gen/toir.cpp index 2f3acc09..da480042 100644 --- a/gen/toir.cpp +++ b/gen/toir.cpp @@ -3253,6 +3253,36 @@ DValue* RemoveExp::toElem(IRState* p) ////////////////////////////////////////////////////////////////////////////////////////// +/// Constructs an array initializer constant with the given constants as its +/// elements. If the element types differ (unions, …), an anonymous struct +/// literal is emitted (as for array constant initializers). +static llvm::Constant* arrayConst(std::vector& vals, + Type* nominalElemType) +{ + if (vals.size() == 0) + { + llvm::ArrayType* type = llvm::ArrayType::get(DtoType(nominalElemType), 0); + return llvm::ConstantArray::get(type, vals); + } + + llvm::Type* elementType = NULL; + bool differentTypes = false; + for (std::vector::iterator i = vals.begin(), end = vals.end(); + i != end; ++i) + { + if (!elementType) + elementType = (*i)->getType(); + else + differentTypes |= (elementType != (*i)->getType()); + } + + if (differentTypes) + return llvm::ConstantStruct::getAnon(vals, true); + + llvm::ArrayType *t = llvm::ArrayType::get(elementType, vals.size()); + return llvm::ConstantArray::get(t, vals); +} + DValue* AssocArrayLiteralExp::toElem(IRState* p) { Logger::print("AssocArrayLiteralExp::toElem: %s @ %s\n", toChars(), type->toChars()); @@ -3307,16 +3337,16 @@ DValue* AssocArrayLiteralExp::toElem(IRState* p) LLConstant* idxs[2] = { DtoConstUint(0), DtoConstUint(0) }; - LLArrayType* arrtype = LLArrayType::get(DtoType(indexType), keys->dim); - LLConstant* initval = LLConstantArray::get(arrtype, keysInits); - LLConstant* globalstore = new LLGlobalVariable(*gIR->module, arrtype, false, LLGlobalValue::InternalLinkage, initval, ".aaKeysStorage"); + LLConstant* initval = arrayConst(keysInits, indexType); + LLConstant* globalstore = new LLGlobalVariable(*gIR->module, initval->getType(), + false, LLGlobalValue::InternalLinkage, initval, ".aaKeysStorage"); LLConstant* slice = llvm::ConstantExpr::getGetElementPtr(globalstore, idxs, true); slice = DtoConstSlice(DtoConstSize_t(keys->dim), slice); LLValue* keysArray = DtoAggrPaint(slice, funcTy->getParamType(1)); - arrtype = LLArrayType::get(DtoType(vtype), values->dim); - initval = LLConstantArray::get(arrtype, valuesInits); - globalstore = new LLGlobalVariable(*gIR->module, arrtype, false, LLGlobalValue::InternalLinkage, initval, ".aaValuesStorage"); + initval = arrayConst(valuesInits, vtype); + globalstore = new LLGlobalVariable(*gIR->module, initval->getType(), + false, LLGlobalValue::InternalLinkage, initval, ".aaValuesStorage"); slice = llvm::ConstantExpr::getGetElementPtr(globalstore, idxs, true); slice = DtoConstSlice(DtoConstSize_t(keys->dim), slice); LLValue* valuesArray = DtoAggrPaint(slice, funcTy->getParamType(2)); From 3ec084da5906ef96c8f2a45a9685395765c50ac6 Mon Sep 17 00:00:00 2001 From: David Nadlinger Date: Thu, 26 Dec 2013 22:48:11 +0100 Subject: [PATCH 6/7] Return undef instead of null on toConstElem failure. This fixes a segfault with associative array literals of arrays of associative array literals, which occured because of arrayLiteralToConst not handling null values properly. Ensuring that null pointers are handled correctly in all toConstElem callers is much more error-prone than just returning an LLVM undef, an error is emitted anyway. The root of the problem is actually in the kludgly implementation of AssocArrayLiteral::toElem, we should revisit this at some point. --- dmd2/expression.h | 8 +++++++- gen/toir.cpp | 27 +++++++++++++++------------ 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/dmd2/expression.h b/dmd2/expression.h index 07816a4f..c436854a 100644 --- a/dmd2/expression.h +++ b/dmd2/expression.h @@ -237,9 +237,15 @@ public: virtual elem *toElem(IRState *irs); elem *toElemDtor(IRState *irs); #if IN_LLVM + /// Emits an LLVM constant corresponding to the expression. + /// + /// Due to the current implementation of AssocArrayLiteralExp::toElem,the + /// implementations have to be able to handle being called on expressions + /// that are not actually constant. In such a case, an LLVM undef of the + /// expected type should be returned (_not_ null). virtual llvm::Constant *toConstElem(IRState *irs); - virtual void cacheLvalue(IRState* irs); + virtual void cacheLvalue(IRState* irs); llvm::Value* cachedLvalue; virtual AssignExp* isAssignExp() { return NULL; } diff --git a/gen/toir.cpp b/gen/toir.cpp index d038753f..9f3b660f 100644 --- a/gen/toir.cpp +++ b/gen/toir.cpp @@ -652,8 +652,8 @@ LLConstant* AddExp::toConstElem(IRState* p) } error("expression '%s' is not a constant", toChars()); - fatal(); - return NULL; + if (!global.gag) fatal(); + return llvm::UndefValue::get(DtoType(type)); } /// Tries to remove a MulExp by a constant value of baseSize from e. Returns @@ -766,8 +766,8 @@ LLConstant* MinExp::toConstElem(IRState* p) } error("expression '%s' is not a constant", toChars()); - fatal(); - return NULL; + if (!global.gag) fatal(); + return llvm::UndefValue::get(DtoType(type)); } DValue* MinExp::toElem(IRState* p) @@ -1231,7 +1231,7 @@ Lerr: error("cannot cast %s to %s at compile time", e1->type->toChars(), type->toChars()); if (!global.gag) fatal(); - return NULL; + return llvm::UndefValue::get(DtoType(type)); } ////////////////////////////////////////////////////////////////////////////////////////// @@ -1290,11 +1290,7 @@ llvm::Constant* SymOffExp::toConstElem(IRState* p) IF_LOG Logger::println("SymOffExp::toConstElem: %s @ %s", toChars(), type->toChars()); LOG_SCOPE; - // We might get null here due to the hackish implementation of - // AssocArrayLiteralExp::toElem. llvm::Constant* base = DtoConstSymbolAddress(loc, var); - if (!base) return 0; - llvm::Constant* result; if (offset == 0) { @@ -1462,7 +1458,8 @@ LLConstant* AddrExp::toConstElem(IRState* p) else if (e1->op == TOKslice) { error("non-constant expression '%s'", toChars()); - fatal(); + if (!global.gag) fatal(); + return llvm::UndefValue::get(DtoType(type)); } // not yet supported else @@ -2872,7 +2869,8 @@ LLConstant* FuncExp::toConstElem(IRState* p) { assert(fd->tok == TOKdelegate || fd->tok == TOKreserved); error("delegate literals as constant expressions are not yet allowed"); - return 0; + if (!global.gag) fatal(); + return llvm::UndefValue::get(DtoType(type)); } // We need to actually codegen the function here, as literals are not added @@ -3557,5 +3555,10 @@ llvm::Constant* Expression::toConstElem(IRState * p) error("expression '%s' is not a constant", toChars()); if (!global.gag) fatal(); - return NULL; + + // Do not return null here, as AssocArrayLiteralExp::toElem determines + // whether it can allocate the needed arrays statically by just invoking + // toConstElem on its key/value expressions, and handling the null value + // consequently would require error-prone adaptions in all other code. + return llvm::UndefValue::get(DtoType(type)); } From 192f3eb13d0dd7d26876af61658565654f7f62ca Mon Sep 17 00:00:00 2001 From: Kai Nacke Date: Thu, 19 Dec 2013 21:45:29 +0100 Subject: [PATCH 7/7] Fix a typo in MSVC build. --- ir/irfuncty.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ir/irfuncty.h b/ir/irfuncty.h index e47b9125..c9ed280a 100644 --- a/ir/irfuncty.h +++ b/ir/irfuncty.h @@ -131,7 +131,7 @@ struct IrFuncTy // Copy constructor and operator= seems to be required for MSC IrFuncTy(const IrFuncTy& rhs) - : funcType(ths.funcType), + : funcType(rhs.funcType), ret(rhs.ret), args(IrFuncTy::ArgList(rhs.args)), arg_sret(rhs.arg_sret),