[mathicgb] 103/393: Apparently GCC does not zero initialize on new T[x]() though it is supposed to. So manual zeroing it is.

[Doug Torrance]

This is an automated email from the git hooks/post-receive script.

dtorrance-guest pushed a commit to branch upstream
in repository mathicgb.

commit 69ed5812abba4ad366bd520e19bcba86b4508a47
Author: Bjarke Hammersholt Roune <bjarkehr.code at gmail.com>
Date:   Mon Nov 5 22:29:45 2012 +0100

    Apparently GCC does not zero initialize on new T[x]() though it is supposed to. So manual zeroing it is.
---
 src/mathicgb/FixedSizeMonomialMap.h | 31 ++++++++++++++++++++++---------
 src/mathicgb/stdinc.h               |  5 -----
 2 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/src/mathicgb/FixedSizeMonomialMap.h b/src/mathicgb/FixedSizeMonomialMap.h
index 57f5340..1166aaa 100755
--- a/src/mathicgb/FixedSizeMonomialMap.h
+++ b/src/mathicgb/FixedSizeMonomialMap.h
@@ -34,10 +34,19 @@ public:
   ):
     mHashToIndexMask(computeHashMask(requestedBucketCount)),
     mBuckets(
-      make_unique_array_init<Atomic<Node*>>(hashMaskToBucketCount(mHashToIndexMask))
+      make_unique_array<Atomic<Node*>>(hashMaskToBucketCount(mHashToIndexMask))
     ),
     mRing(ring),
-    mNodeAlloc(sizeofNode(ring)) {}
+    mNodeAlloc(sizeofNode(ring))
+  {
+    // Calling new int[x] does not zero the array. std::atomic has a trivial
+    // constructor so the same thing is true of new atomic[x]. Calling
+    // new int[x]() is supposed to zero initialize but this apparently
+    // does not work on GCC. So we have to fill the table with nulls
+    // manually. This was wonderful to debug btw.
+    // We can store relaxed as the constructor does not run concurrently.
+    setTableEntriesToNullRelaxed();
+  }
 
   /// Construct a hash table with at least requestedBucketCount buckets and
   /// insert the elements from the parameter map.
@@ -53,11 +62,13 @@ public:
   ):
     mHashToIndexMask(computeHashMask(requestedBucketCount)),
     mBuckets(
-      make_unique_array_init<Atomic<Node*>>(hashMaskToBucketCount(mHashToIndexMask))
+      make_unique_array<Atomic<Node*>>(hashMaskToBucketCount(mHashToIndexMask))
     ),
     mRing(map.ring()),
     mNodeAlloc(std::move(map.mNodeAlloc))
   {
+    // We can store relaxed as the constructor does not run concurrently.
+    setTableEntriesToNullRelaxed();
     const auto tableEnd = map.mBuckets.get() + map.bucketCount();
     for (auto tableIt = map.mBuckets.get(); tableIt != tableEnd; ++tableIt) {
       for (Node* node = tableIt->load(); node != 0;) {
@@ -181,12 +192,8 @@ public:
       MATHICGB_ASSERT(false);
     }
 #endif
-    const auto tableEnd = mBuckets.get() + bucketCount();
-    for (auto tableIt = mBuckets.get(); tableIt != tableEnd; ++tableIt) {
-      // we can store as std::memory_order_relaxed because the client is
-      // already required to ensure synchronization.
-      tableIt->store(0, std::memory_order_relaxed);
-    }
+    // we can store relaxed as the client supplies synchronization.
+    setTableEntriesToNullRelaxed();
 
     // This is the reason that we cannot support this operation concurrently -
     // we have no way to know when it is safe to deallocate the monomials
@@ -195,6 +202,12 @@ public:
   }
 
 private:
+  void setTableEntriesToNullRelaxed() {
+    const auto tableEnd = mBuckets.get() + bucketCount();
+    for (auto tableIt = mBuckets.get(); tableIt != tableEnd; ++tableIt)
+      tableIt->store(0, std::memory_order_relaxed);
+  }
+
   struct Node {
     Node(Node* next, const mapped_type value): next(next), value(value) {}
 
diff --git a/src/mathicgb/stdinc.h b/src/mathicgb/stdinc.h
index db5c4dc..0c5b233 100755
--- a/src/mathicgb/stdinc.h
+++ b/src/mathicgb/stdinc.h
@@ -233,11 +233,6 @@ std::unique_ptr<T[]> make_unique_array(size_t count) {
   return std::unique_ptr<T[]>(new T[count]);
 }
 
-template<class T>
-std::unique_ptr<T[]> make_unique_array_init(size_t count) {
-  return std::unique_ptr<T[]>(new T[count]());
-}
-
 // TODO: These types should be defined in some way that actually
 // checks that these bit counts are right like in a configure script.
 typedef unsigned long long uint64;

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/debian-science/packages/mathicgb.git