[pkg-d-commits] [ldc] 106/211: Fix union layout and initialization
Matthias Klumpp
mak at moszumanska.debian.org
Sun Apr 23 22:36:14 UTC 2017
This is an automated email from the git hooks/post-receive script.
mak pushed a commit to annotated tag v1.1.0
in repository ldc.
commit e6537ba4dcdd8e80a4ba753e089bfc0833a16c9c
Author: Martin <noone at nowhere.com>
Date: Fri Oct 21 03:32:58 2016 +0200
Fix union layout and initialization
By refactoring IrAggr::addFieldInitializers() and making it use an
extended and refactored AggrTypeBuilder::addAggregate().
AggrTypeBuilder::addAggregate() can now optionally detect alias fields
in unions (same offset and LL type as a dominant union field) and add
those to the variable-to-GEP-index mapping.
These alias fields can then be indexed directly with a GEP instead of
resorting to casting the pointer and applying the byte offset.
---
ir/iraggr.cpp | 163 ++++++++--------------------------------------------
ir/irtypeaggr.cpp | 168 ++++++++++++++++++++++++++----------------------------
ir/irtypeaggr.h | 14 ++++-
3 files changed, 116 insertions(+), 229 deletions(-)
diff --git a/ir/iraggr.cpp b/ir/iraggr.cpp
index 72de3d7..90d194b 100644
--- a/ir/iraggr.cpp
+++ b/ir/iraggr.cpp
@@ -92,13 +92,6 @@ add_zeros(llvm::SmallVectorImpl<llvm::Constant *> &constants,
//////////////////////////////////////////////////////////////////////////////
//////////////////////////////////////////////////////////////////////////////
-typedef std::pair<VarDeclaration *, llvm::Constant *> VarInitConst;
-
-static bool struct_init_data_sort(const VarInitConst &a,
- const VarInitConst &b) {
- return (a.first && b.first) ? a.first->offset < b.first->offset : false;
-}
-
// helper function that returns the static default initializer of a variable
LLConstant *get_default_initializer(VarDeclaration *vd) {
if (vd->_init) {
@@ -171,9 +164,8 @@ IrAggr::createInitializerConstant(const VarInitMap &explicitInitializers,
// tail padding?
const size_t structsize = aggrdecl->size(Loc());
- if (offset < structsize) {
+ if (offset < structsize)
add_zeros(constants, offset, structsize);
- }
// get initializer type
if (!initializerType || initializerType->isOpaque()) {
@@ -221,153 +213,44 @@ void IrAggr::addFieldInitializers(
bool newinsts = (cd == aggrdecl->isClassDeclaration());
size_t inter_idx = interfacesWithVtbls.size();
-
- offset = (offset + Target::ptrsize - 1) & ~(Target::ptrsize - 1);
-
for (auto bc : *cd->vtblInterfaces) {
constants.push_back(getInterfaceVtbl(bc, newinsts, inter_idx));
offset += Target::ptrsize;
inter_idx++;
- if (populateInterfacesWithVtbls) {
+ if (populateInterfacesWithVtbls)
interfacesWithVtbls.push_back(bc);
- }
}
}
}
- // Build up vector with one-to-one mapping to field indices.
- const size_t n = decl->fields.dim;
- llvm::SmallVector<VarInitConst, 16> data(n);
-
- // Fill in explicit initializers.
- for (size_t i = 0; i < n; ++i) {
- VarDeclaration *vd = decl->fields[i];
- auto expl = explicitInitializers.find(vd);
- if (expl != explicitInitializers.end()) {
- const unsigned vd_begin = vd->offset;
- const unsigned vd_end = vd_begin + vd->type->size();
-
- // Make sure it doesn't overlap any prior initializers (needed for
- // unions). This effectively initializes only the first member with an
- // explicit initializer of a union.
- // Only classes and structs can contain unions / overlapping fields.
- if (type->ty == Tstruct || type->ty == Tclass) {
- bool overlaps = false;
- for (size_t j = 0; j < i; ++j) {
- if (!data[j].first) {
- continue;
- }
-
- const unsigned f_begin = decl->fields[j]->offset;
- const unsigned f_end = f_begin + decl->fields[j]->type->size();
- if (vd_begin >= f_end || vd_end <= f_begin) {
- continue;
- }
-
- overlaps = true;
- break;
- }
- if (overlaps)
- continue;
- }
-
- data[i] = *expl;
- }
- }
-
- // Fill in implicit initializers
- for (size_t i = 0; i < n; i++) {
- if (data[i].first) {
- continue;
- }
+ AggrTypeBuilder b(false, offset);
+ b.addAggregate(decl, &explicitInitializers, AggrTypeBuilder::Aliases::Skip);
+ offset = b.currentOffset();
- VarDeclaration *vd = decl->fields[i];
-
- /* Skip void initializers for unions. DMD bug 3991:
- union X
- {
- int a = void;
- dchar b = 'a';
- }
- */
- // FIXME: decl->isUnionDeclaration() is always false, the FE lowers
- // UnionDeclarations.
- if (decl->isUnionDeclaration() && vd->_init &&
- vd->_init->isVoidInitializer()) {
- continue;
- }
+ const size_t baseLLFieldIndex = constants.size();
+ const size_t numNewLLFields = b.defaultTypes().size();
+ constants.resize(constants.size() + numNewLLFields, nullptr);
- unsigned vd_begin = vd->offset;
- unsigned vd_end = vd_begin + vd->type->size();
+ // add explicit and non-overlapping implicit initializers
+ for (const auto &pair : b.varGEPIndices()) {
+ const auto field = pair.first;
+ const size_t fieldIndex = pair.second;
- /* Skip zero size fields like zero-length static arrays, LDC issue 812:
- class B {
- ubyte[0] test;
- }
- */
- if (vd_begin == vd_end) {
- continue;
- }
+ const auto explicitIt = explicitInitializers.find(field);
+ llvm::Constant *init = (explicitIt != explicitInitializers.end()
+ ? explicitIt->second
+ : get_default_initializer(field));
- // make sure it doesn't overlap any explicit initializers.
- bool overlaps = false;
- if (type->ty == Tstruct || type->ty == Tclass) {
- // Only classes and structs can have overlapping fields.
- for (size_t j = 0; j < n; ++j) {
- if (i == j || !data[j].first) {
- continue;
- }
-
- VarDeclaration *it = decl->fields[j];
- unsigned f_begin = it->offset;
- unsigned f_end = f_begin + it->type->size();
-
- if (vd_begin >= f_end || vd_end <= f_begin) {
- continue;
- }
-
- overlaps = true;
- break;
- }
- }
- // add if no overlap found
- if (!overlaps) {
- IF_LOG Logger::println("Implicit initializer: %s @+%u", vd->toChars(),
- vd->offset);
- LOG_SCOPE;
-
- data[i].first = vd;
- data[i].second = get_default_initializer(vd);
- }
+ constants[baseLLFieldIndex + fieldIndex] =
+ FillSArrayDims(field->type, init);
}
- // Sort data array by offset.
- // TODO: Figure out whether this is really necessary, fields should already
- // be in offset order. Not having do do this would mean we could use a plain
- // llvm::Constant* vector for initializers and avoid all the VarInitConst
- // business.
- std::sort(data.begin(), data.end(), struct_init_data_sort);
-
- // build array of constants and make sure explicit zero padding is inserted
- // when necessary.
- for (size_t i = 0; i < n; i++) {
- VarDeclaration *vd = data[i].first;
- if (vd == nullptr) {
- continue;
- }
-
- // Explicitly zero the padding as per TDPL §7.1.1. Otherwise, it would
- // be left uninitialized by LLVM.
- if (offset < vd->offset) {
- add_zeros(constants, offset, vd->offset);
- offset = vd->offset;
- }
-
- IF_LOG Logger::println("adding field %s", vd->toChars());
-
- constants.push_back(FillSArrayDims(vd->type, data[i].second));
- offset += getMemberSize(vd->type);
+ // zero out remaining padding fields
+ for (size_t i = 0; i < numNewLLFields; i++) {
+ auto &init = constants[baseLLFieldIndex + i];
+ if (!init)
+ init = llvm::Constant::getNullValue(b.defaultTypes()[i]);
}
}
diff --git a/ir/irtypeaggr.cpp b/ir/irtypeaggr.cpp
index 94baef7..50f8092 100644
--- a/ir/irtypeaggr.cpp
+++ b/ir/irtypeaggr.cpp
@@ -42,7 +42,8 @@ bool var_offset_sort_cb(const VarDeclaration *v1, const VarDeclaration *v2) {
return v1 && !v2;
}
-AggrTypeBuilder::AggrTypeBuilder(bool packed) : m_packed(packed) {
+AggrTypeBuilder::AggrTypeBuilder(bool packed, unsigned offset)
+ : m_packed(packed), m_offset(offset) {
m_defaultTypes.reserve(32);
}
@@ -53,110 +54,100 @@ void AggrTypeBuilder::addType(llvm::Type *type, unsigned size) {
}
void AggrTypeBuilder::addAggregate(AggregateDeclaration *ad) {
- // mirror the ad->fields array but only fill in contributors
+ addAggregate(ad, nullptr, Aliases::AddToVarGEPIndices);
+}
+
+namespace {
+enum FieldPriority {
+ FP_ExplicitVoid = 0, // lowest priority: fields with explicit void initializer
+ FP_Default = 1, // default initializer
+ FP_Explicit = 2, // explicit non-void initializer
+ FP_Value = 3, // highest priority: values (for literals)
+};
+
+FieldPriority prioritize(VarDeclaration *field,
+ const AggrTypeBuilder::VarInitMap *explicitInits) {
+ if (explicitInits && explicitInits->find(field) != explicitInits->end())
+ return FP_Value;
+ if (auto init = field->_init)
+ return !init->isVoidInitializer() ? FP_Explicit : FP_ExplicitVoid;
+ return FP_Default;
+}
+}
+
+void AggrTypeBuilder::addAggregate(
+ AggregateDeclaration *ad, const AggrTypeBuilder::VarInitMap *explicitInits,
+ AggrTypeBuilder::Aliases aliases) {
const size_t n = ad->fields.dim;
+ if (n == 0)
+ return;
+
+ // prioritize overlapping fields
+ LLSmallVector<FieldPriority, 16> priorities;
+ priorities.reserve(n);
+ for (auto f : ad->fields) {
+ priorities.push_back(prioritize(f, explicitInits));
+ IF_LOG Logger::println("Field priority for %s: %d", f->toChars(),
+ priorities.back());
+ }
+
+ // mirror the ad->fields array but only fill in contributors
LLSmallVector<VarDeclaration *, 16> data(n, nullptr);
- unsigned int errors = global.errors;
+ // list of pairs: alias => actual field (same offset, same LL type)
+ LLSmallVector<std::pair<VarDeclaration *, VarDeclaration *>, 16> aliasPairs;
- // first fill in the fields with explicit initializers
- for (size_t index = 0; index < n; ++index) {
- VarDeclaration *field = ad->fields[index];
+ // one pass per priority in descending order
+ const auto minMaxPriority =
+ std::minmax_element(priorities.begin(), priorities.end());
+ for (int p = *minMaxPriority.second; p >= *minMaxPriority.first; p--) {
+ // iterate over fields of that priority, in declaration order
+ for (size_t index = 0; index < n; ++index) {
+ if (priorities[index] != p)
+ continue;
- // _init is !null for explicit inits
- if (field->_init != nullptr && !field->_init->isVoidInitializer()) {
- IF_LOG Logger::println("adding explicit initializer for struct field %s",
- field->toChars());
+ VarDeclaration *field = ad->fields[index];
+ const size_t f_begin = field->offset;
+ const size_t f_end = f_begin + field->type->size();
- size_t f_size = field->type->size();
- size_t f_begin = field->offset;
- size_t f_end = f_begin + f_size;
- if (f_size == 0) {
+ // skip empty fields
+ if (f_begin == f_end)
continue;
- }
- data[index] = field;
+ // check for overlapping existing fields
+ bool overlaps = false;
+ for (const auto vd : data) {
+ if (!vd)
+ continue;
- // make sure there is no overlap
- for (size_t i = 0; i < index; i++) {
- if (data[i] != nullptr) {
- VarDeclaration *vd = data[i];
- size_t v_begin = vd->offset;
- size_t v_end = v_begin + vd->type->size();
+ const size_t v_begin = vd->offset;
+ const size_t v_end = v_begin + vd->type->size();
- if (v_begin >= f_end || v_end <= f_begin) {
- continue;
+ if (v_begin < f_end && v_end > f_begin) {
+ if (aliases == Aliases::AddToVarGEPIndices && v_begin == f_begin &&
+ DtoMemType(vd->type) == DtoMemType(field->type)) {
+ aliasPairs.push_back(std::make_pair(field, vd));
}
-
- ad->error(vd->loc, "has overlapping initialization for %s and %s",
- field->toChars(), vd->toChars());
+ overlaps = true;
+ break;
}
}
- }
- }
- if (errors != global.errors) {
- // There was an overlapping initialization.
- // Return if errors are gagged otherwise abort.
- if (global.gag) {
- return;
+ if (!overlaps)
+ data[index] = field;
}
- fatal();
}
- // fill in default initializers
- for (size_t index = 0; index < n; ++index) {
- if (data[index]) {
- continue;
- }
- VarDeclaration *field = ad->fields[index];
-
- size_t f_size = field->type->size();
- size_t f_begin = field->offset;
- size_t f_end = f_begin + f_size;
- if (f_size == 0) {
- continue;
- }
-
- // make sure it doesn't overlap anything explicit
- bool overlaps = false;
- for (size_t i = 0; i < n; i++) {
- if (data[i]) {
- size_t v_begin = data[i]->offset;
- size_t v_end = v_begin + data[i]->type->size();
-
- if (v_begin >= f_end || v_end <= f_begin) {
- continue;
- }
-
- overlaps = true;
- break;
- }
- }
-
- // if no overlap was found, add the default initializer
- if (!overlaps) {
- IF_LOG Logger::println("adding default initializer for struct field %s",
- field->toChars());
- data[index] = field;
- }
- }
-
- //
- // ok. now we can build a list of llvm types. and make sure zeros are inserted
- // if necessary.
- //
+ // Now we can build a list of LLVM types for the actual LL fields.
+ // Make sure to zero out any padding and set the GEP indices for the directly
+ // indexable variables.
// first we sort the list by offset
std::sort(data.begin(), data.end(), var_offset_sort_cb);
- // add types to list
- for (size_t i = 0; i < n; i++) {
- VarDeclaration *vd = data[i];
-
- if (vd == nullptr) {
+ for (const auto vd : data) {
+ if (!vd)
continue;
- }
assert(vd->offset >= m_offset && "Variable overlaps previous field.");
@@ -175,6 +166,13 @@ void AggrTypeBuilder::addAggregate(AggregateDeclaration *ad) {
// set the field index
m_varGEPIndices[vd] = m_fieldIndex;
+
+ // let any aliases reuse this field/GEP index
+ for (const auto &pair : aliasPairs) {
+ if (pair.second == vd)
+ m_varGEPIndices[pair.first] = m_fieldIndex;
+ }
+
++m_fieldIndex;
}
}
@@ -190,10 +188,8 @@ void AggrTypeBuilder::alignCurrentOffset(unsigned alignment) {
}
void AggrTypeBuilder::addTailPadding(unsigned aggregateSize) {
- // tail padding?
- if (m_offset < aggregateSize) {
+ if (m_offset < aggregateSize)
add_zeros(m_defaultTypes, m_offset, aggregateSize);
- }
}
//////////////////////////////////////////////////////////////////////////////
diff --git a/ir/irtypeaggr.h b/ir/irtypeaggr.h
index f45579c..b799dc3 100644
--- a/ir/irtypeaggr.h
+++ b/ir/irtypeaggr.h
@@ -28,15 +28,23 @@ using VarGEPIndices = std::map<VarDeclaration *, unsigned>;
class AggrTypeBuilder {
public:
- explicit AggrTypeBuilder(bool packed);
+ using VarInitMap = std::map<VarDeclaration *, llvm::Constant *>;
+ enum class Aliases { Skip, AddToVarGEPIndices };
+
+ explicit AggrTypeBuilder(bool packed, unsigned offset = 0);
void addType(llvm::Type *type, unsigned size);
void addAggregate(AggregateDeclaration *ad);
+ void addAggregate(AggregateDeclaration *ad, const VarInitMap *explicitInits,
+ Aliases aliases);
void alignCurrentOffset(unsigned alignment);
void addTailPadding(unsigned aggregateSize);
unsigned currentFieldIndex() const { return m_fieldIndex; }
- std::vector<llvm::Type *> defaultTypes() const { return m_defaultTypes; }
- VarGEPIndices varGEPIndices() const { return m_varGEPIndices; }
+ const std::vector<llvm::Type *> &defaultTypes() const {
+ return m_defaultTypes;
+ }
+ const VarGEPIndices &varGEPIndices() const { return m_varGEPIndices; }
unsigned overallAlignment() const { return m_overallAlignment; }
+ unsigned currentOffset() const { return m_offset; }
protected:
std::vector<llvm::Type *> m_defaultTypes;
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-d/ldc.git
More information about the pkg-d-commits
mailing list