Subzero: Fix stats collection and output for multithreading.

src/IceGlobalContext.h

Index: src/IceGlobalContext.h
diff --git a/src/IceGlobalContext.h b/src/IceGlobalContext.h
index 04f08a2dbffa3191633fb39343b9e6e03f1ef1e0..fe5d163ec5638b46b45895a79d07db922676b85a 100644
--- a/src/IceGlobalContext.h
+++ b/src/IceGlobalContext.h
@@ -62,25 +62,39 @@ class GlobalContext {
   class CodeStats {
     CodeStats(const CodeStats &) = delete;
     CodeStats &operator=(const CodeStats &) = default;
+#define CODESTATS_TABLE                                                        \
+  /* dump string, enum value */                                                \
+  X("Inst Count  ", InstCount)                                                 \
+  X("Regs Saved  ", RegsSaved)                                                 \
+  X("Frame Bytes ", FrameByte)                                                 \
+  X("Spills      ", NumSpills)                                                 \
+  X("Fills       ", NumFills)
+    //#define X(str, tag)

[JF, 2015/01/31 17:46:25]

Leftover?

[Jim Stichnoth, 2015/01/31 18:32:29]

No - our x-macros style when defining the table is to leave the expected
"#define X(...)" usage in a comment.  I just wish clang-format wouldn't indent
it...

 
   public:
-    CodeStats()
-        : InstructionsEmitted(0), RegistersSaved(0), FrameBytes(0), Spills(0),
-          Fills(0) {}
-    void reset() { *this = CodeStats(); }
-    void updateEmitted(uint32_t InstCount) { InstructionsEmitted += InstCount; }
-    void updateRegistersSaved(uint32_t Num) { RegistersSaved += Num; }
-    void updateFrameBytes(uint32_t Bytes) { FrameBytes += Bytes; }
-    void updateSpills() { ++Spills; }
-    void updateFills() { ++Fills; }
+    enum CSTag {
+#define X(str, tag) CS_##tag,
+      CODESTATS_TABLE
+#undef X
+          CS_NUM
+    };
+    CodeStats() { reset(); }
+    void reset() {
+      for (uint32_t i = 0; i < CS_NUM; ++i)
+        Stats[i] = 0;
+    }
+    void update(CSTag Tag, uint32_t Count = 1) {
+      assert(Tag < CS_NUM);
+      Stats[Tag] += Count;
+    }
+    void add(const CodeStats &Other) {
+      for (uint32_t i = 0; i < CS_NUM; ++i)
+        Stats[i] += Other.Stats[i];
+    }
     void dump(const IceString &Name, Ostream &Str);
 
   private:
-    uint32_t InstructionsEmitted;
-    uint32_t RegistersSaved;
-    uint32_t FrameBytes;
-    uint32_t Spills;
-    uint32_t Fills;
+    uint32_t Stats[CS_NUM];

[JF, 2015/01/31 17:46:25]

std::array, and then you can use the std algorithms above :-D

[Jim Stichnoth, 2015/01/31 18:32:29]

Done.

   };
 
   // TimerList is a vector of TimerStack objects, with extra methods
@@ -121,6 +135,7 @@ class GlobalContext {
   public:
     ThreadContext() {}
     CodeStats StatsFunction;
+    CodeStats StatsCumulative;
     TimerList Timers;
   };
 
@@ -209,32 +224,37 @@ public:
   void statsUpdateEmitted(uint32_t InstCount) {
     if (!ALLOW_DUMP || !getFlags().DumpStats)
       return;
-    ICE_TLS_GET_FIELD(TLS)->StatsFunction.updateEmitted(InstCount);
-    getStatsCumulative()->updateEmitted(InstCount);
+    ThreadContext *TLS = ICE_TLS_GET_FIELD(TLS);
+    TLS->StatsFunction.update(CodeStats::CS_InstCount, InstCount);
+    TLS->StatsCumulative.update(CodeStats::CS_InstCount, InstCount);
   }
   void statsUpdateRegistersSaved(uint32_t Num) {
     if (!ALLOW_DUMP || !getFlags().DumpStats)
       return;
-    ICE_TLS_GET_FIELD(TLS)->StatsFunction.updateRegistersSaved(Num);
-    getStatsCumulative()->updateRegistersSaved(Num);
+    ThreadContext *TLS = ICE_TLS_GET_FIELD(TLS);
+    TLS->StatsFunction.update(CodeStats::CS_RegsSaved, Num);
+    TLS->StatsCumulative.update(CodeStats::CS_RegsSaved, Num);
   }
   void statsUpdateFrameBytes(uint32_t Bytes) {
     if (!ALLOW_DUMP || !getFlags().DumpStats)
       return;
-    ICE_TLS_GET_FIELD(TLS)->StatsFunction.updateFrameBytes(Bytes);
-    getStatsCumulative()->updateFrameBytes(Bytes);
+    ThreadContext *TLS = ICE_TLS_GET_FIELD(TLS);
+    TLS->StatsFunction.update(CodeStats::CS_FrameByte, Bytes);
+    TLS->StatsCumulative.update(CodeStats::CS_FrameByte, Bytes);
   }
   void statsUpdateSpills() {
     if (!ALLOW_DUMP || !getFlags().DumpStats)
       return;
-    ICE_TLS_GET_FIELD(TLS)->StatsFunction.updateSpills();
-    getStatsCumulative()->updateSpills();
+    ThreadContext *TLS = ICE_TLS_GET_FIELD(TLS);
+    TLS->StatsFunction.update(CodeStats::CS_NumSpills);
+    TLS->StatsCumulative.update(CodeStats::CS_NumSpills);
   }
   void statsUpdateFills() {
     if (!ALLOW_DUMP || !getFlags().DumpStats)
       return;
-    ICE_TLS_GET_FIELD(TLS)->StatsFunction.updateFills();
-    getStatsCumulative()->updateFills();
+    ThreadContext *TLS = ICE_TLS_GET_FIELD(TLS);
+    TLS->StatsFunction.update(CodeStats::CS_NumFills);
+    TLS->StatsCumulative.update(CodeStats::CS_NumFills);
   }
 
   // These are predefined TimerStackIdT values.
@@ -296,6 +316,13 @@ public:
       for (ThreadContext *TLS : AllThreadContexts)
         Timers->mergeFrom(TLS->Timers);
     }
+    if (ALLOW_DUMP) {
+      // Do a separate loop over AllThreadContexts to avoid holding
+      // two locks at once.
+      auto Stats = getStatsCumulative();
+      for (ThreadContext *TLS : AllThreadContexts)
+        Stats->add(TLS->StatsCumulative);
+    }
   }
 
   // Translation thread startup routine.