Subzero: Emit functions and global initializers in a separate thread.

src/IceThreading.h

Index: src/IceThreading.h
diff --git a/src/IceUtils.h b/src/IceThreading.h
similarity index 63%
copy from src/IceUtils.h
copy to src/IceThreading.h
index dc3a5ff1d77ef160d2b321038f17db3ed64cdbde..3ded713853e3d2cb98d3b9db3b55588e04640ccb 100644
--- a/src/IceUtils.h
+++ b/src/IceThreading.h
@@ -1,4 +1,4 @@
-//===- subzero/src/IceUtils.h - Utility functions ---------------*- C++ -*-===//
+//===- subzero/src/IceThreading.h - Threading functions ---------*- C++ -*-===//
 //
 //                        The Subzero Code Generator
 //
@@ -7,61 +7,19 @@
 //
 //===----------------------------------------------------------------------===//
 //
-// This file declares some utility functions.
+// This file defines threading-related functions.
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef SUBZERO_SRC_ICEUTILS_H
-#define SUBZERO_SRC_ICEUTILS_H
+#ifndef SUBZERO_SRC_ICETHREADING_H
+#define SUBZERO_SRC_ICETHREADING_H
 
-#include <climits>
 #include <condition_variable>
+#include <mutex>
 
-namespace Ice {
-
-// Similar to bit_cast, but allows copying from types of unrelated
-// sizes. This method was introduced to enable the strict aliasing
-// optimizations of GCC 4.4. Basically, GCC mindlessly relies on
-// obscure details in the C++ standard that make reinterpret_cast
-// virtually useless.
-template <class D, class S> inline D bit_copy(const S &source) {
-  D destination;
-  // This use of memcpy is safe: source and destination cannot overlap.
-  memcpy(&destination, reinterpret_cast<const void *>(&source),
-         sizeof(destination));
-  return destination;
-}
-
-class Utils {
-public:
-  // Check whether an N-bit two's-complement representation can hold value.
-  template <typename T> static inline bool IsInt(int N, T value) {
-    assert((0 < N) &&
-           (static_cast<unsigned int>(N) < (CHAR_BIT * sizeof(value))));
-    T limit = static_cast<T>(1) << (N - 1);
-    return (-limit <= value) && (value < limit);
-  }
-
-  template <typename T> static inline bool IsUint(int N, T value) {
-    assert((0 < N) &&
-           (static_cast<unsigned int>(N) < (CHAR_BIT * sizeof(value))));
-    T limit = static_cast<T>(1) << N;
-    return (0 <= value) && (value < limit);
-  }
-
-  template <typename T> static inline bool WouldOverflowAdd(T X, T Y) {
-    return ((X > 0 && Y > 0 && (X > std::numeric_limits<T>::max() - Y)) ||
-            (X < 0 && Y < 0 && (X < std::numeric_limits<T>::min() - Y)));
-  }
+#include "IceDefs.h"
 
-  static inline uint64_t OffsetToAlignment(uint64_t Pos, uint64_t Align) {
-    assert(llvm::isPowerOf2_64(Align));
-    uint64_t Mod = Pos & (Align - 1);
-    if (Mod == 0)
-      return 0;
-    return Align - Mod;
-  }
-};
+namespace Ice {
 
 // BoundedProducerConsumerQueue is a work queue that allows multiple
 // producers and multiple consumers.  A producer adds entries using
@@ -96,8 +54,11 @@ class BoundedProducerConsumerQueue {
   operator=(const BoundedProducerConsumerQueue &) = delete;
 
 public:
-  BoundedProducerConsumerQueue(size_t MaxSize, bool Sequential)
-      : Back(0), Front(0), MaxSize(std::min(MaxSize, MaxStaticSize)),
+  const static size_t UnlimitedSize = 0;
+  BoundedProducerConsumerQueue(bool Sequential, size_t MaxSize = UnlimitedSize)
+      : Back(0), Front(0),
+        MaxSize(MaxSize == UnlimitedSize ? MaxStaticSize
+                                         : std::min(MaxSize, MaxStaticSize)),

[JF, 2015/02/08 21:15:04]

Why have UnlimitedSize at all when MaxStaticSize would do the same?

[Jim Stichnoth, 2015/02/10 07:51:47]

Oh, right!  Done.

         Sequential(Sequential), IsEnded(false) {}
   void blockingPush(T *Item) {
     {
@@ -190,6 +151,69 @@ private:
   }
 };
 
+class EmitterWorkItem {

[JF, 2015/02/08 21:15:05]

Overall this class looks like it wants to be a virtual class (with all the
different ctors and all) but is instead the union of all the derived class'
functionality. IIUC you want it all inline without abstraction to ensure that it
can be held in datastructures without indirection and extra allocations?

That's fine with me, I just want to make sure that's intentional, and if so you
should document the design choice on the class.

[Jim Stichnoth, 2015/02/10 07:51:47]

In some world, I would have Cfg, Assembler, and VariableDeclarationList all
inheriting from the EmitterWorkItem interface.  EmitterWorkItem would hold the
Sequence and Kind fields (or Kind would be separately derived from some RTTI
mechanism), and the appropriate get() methods would just be static casts.

Or, EmitterWorkItem could be a tagged union plus the Sequence field, perhaps
with a nice destructor defined in the tagged union.

These ideas have a certain ickiness that led to the slightly wasteful approach
here, where at most one of the pointer fields is non-null.

I don't care too much about inline versus non-inline since there are relatively
few operations, but it's simple enough that it's nice to define in just one
place.

Added some documentation above.

+  EmitterWorkItem() = delete;
+  EmitterWorkItem(const EmitterWorkItem &) = delete;
+  EmitterWorkItem &operator=(const EmitterWorkItem &) = delete;
+
+public:
+  // ItemKind can be one of the following:
+  //
+  // WI_Nop: No actual work.  This is a placeholder to maintain
+  // sequence numbers in case there is a translation error.
+  //
+  // WI_GlobalInits: A list of global declarations and initializers.
+  //
+  // WI_Asm: A function that has already had emitIAS() called on it.
+  // The work is transferred via the Assembler buffer, and the
+  // originating Cfg has been deleted (to recover lots of memory).
+  //
+  // WI_Cfg: A Cfg that has not yet had emit() or emitIAS() called on
+  // it.  This is only used as a debugging configuration when we want
+  // to emit "readable" assembly code, possibly annotated with
+  // liveness and other information only available in the Cfg and not
+  // in the Assembler buffer.
+  enum ItemKind { WI_Nop, WI_GlobalInits, WI_Asm, WI_Cfg };
+  // Constructor for a WI_Nop work item.
+  explicit EmitterWorkItem(uint32_t Seq)
+      : Sequence(Seq), Kind(WI_Nop), GlobalInits(nullptr), Function(nullptr),
+        RawFunc(nullptr) {}
+  // Constructor for a WI_GlobalInits work item.
+  EmitterWorkItem(uint32_t Seq, VariableDeclarationList *D)
+      : Sequence(Seq), Kind(WI_GlobalInits), GlobalInits(D), Function(nullptr),
+        RawFunc(nullptr) {}
+  // Constructor for a WI_Asm work item.
+  EmitterWorkItem(uint32_t Seq, Assembler *A)
+      : Sequence(Seq), Kind(WI_Asm), GlobalInits(nullptr), Function(A),
+        RawFunc(nullptr) {}
+  // Constructor for a WI_Cfg work item.
+  EmitterWorkItem(uint32_t Seq, Cfg *F)
+      : Sequence(Seq), Kind(WI_Cfg), GlobalInits(nullptr), Function(nullptr),
+        RawFunc(F) {}
+  uint32_t getSequenceNumber() const { return Sequence; }
+  ItemKind getKind() const { return Kind; }
+  VariableDeclarationList *getGlobalInits() const {
+    assert(getKind() == WI_GlobalInits);
+    return GlobalInits;
+  }
+  Assembler *getAsm() const {
+    assert(getKind() == WI_Asm);
+    return Function;
+  }
+  Cfg *getCfg() const {
+    assert(getKind() == WI_Cfg);
+    return RawFunc;
+  }
+  ~EmitterWorkItem();
+
+private:
+  const uint32_t Sequence;
+  const ItemKind Kind;
+  VariableDeclarationList *const GlobalInits;
+  Assembler *const Function;
+  Cfg *const RawFunc;
+};
+
 } // end of namespace Ice
 
-#endif // SUBZERO_SRC_ICEUTILS_H
+#endif // SUBZERO_SRC_ICETHREADING_H