[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