From 31da130e4dcce5a4fe72eb187db3e169cba4ec23 Mon Sep 17 00:00:00 2001
From: George Burgess IV <george.burgess.iv@gmail.com>
Date: Thu, 2 Aug 2018 19:50:27 +0000
Subject: [PATCH] [Support] Add an enable bit to our DebugCounters

r337748 made us start incrementing DebugCounters all of the time. This
makes tsan unhappy in multithreaded environments.

Since it doesn't make much sense to use DebugCounters with multiple
threads, this patch makes us only count anything if the user passed a
-debug-counter option or if some other piece of code explicitly asks
for it (e.g. the pass in D50031).

The amount of global state here makes writing a unittest for this
behavior somewhat awkward. So, no test is provided.

Differential Revision: https://reviews.llvm.org/D50150

llvm-svn: 338762
---
 llvm/include/llvm/Support/DebugCounter.h | 28 +++++++++++++++++++-----
 llvm/lib/Support/DebugCounter.cpp        |  2 ++
 2 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/llvm/include/llvm/Support/DebugCounter.h b/llvm/include/llvm/Support/DebugCounter.h
index 250fc6bb1f5cf..83bd5a06c94a1 100644
--- a/llvm/include/llvm/Support/DebugCounter.h
+++ b/llvm/include/llvm/Support/DebugCounter.h
@@ -70,10 +70,9 @@ class DebugCounter {
     return instance().addCounter(Name, Desc);
   }
   inline static bool shouldExecute(unsigned CounterName) {
-// Compile to nothing when debugging is off
-#ifdef NDEBUG
-    return true;
-#else
+    if (!isCountingEnabled())
+      return true;
+
     auto &Us = instance();
     auto Result = Us.Counters.find(CounterName);
     if (Result != Us.Counters.end()) {
@@ -93,7 +92,6 @@ class DebugCounter {
     }
     // Didn't find the counter, should we warn?
     return true;
-#endif // NDEBUG
   }
 
   // Return true if a given counter had values set (either programatically or on
@@ -142,7 +140,23 @@ class DebugCounter {
   }
   CounterVector::const_iterator end() const { return RegisteredCounters.end(); }
 
+  // Force-enables counting all DebugCounters.
+  //
+  // Since DebugCounters are incompatible with threading (not only do they not
+  // make sense, but we'll also see data races), this should only be used in
+  // contexts where we're certain we won't spawn threads.
+  static void enableAllCounters() { instance().Enabled = true; }
+
 private:
+  static bool isCountingEnabled() {
+// Compile to nothing when debugging is off
+#ifdef NDEBUG
+    return false;
+#else
+    return instance().Enabled;
+#endif
+  }
+
   unsigned addCounter(const std::string &Name, const std::string &Desc) {
     unsigned Result = RegisteredCounters.insert(Name);
     Counters[Result] = {};
@@ -159,6 +173,10 @@ class DebugCounter {
   };
   DenseMap<unsigned, CounterInfo> Counters;
   CounterVector RegisteredCounters;
+
+  // Whether we should do DebugCounting at all. DebugCounters aren't
+  // thread-safe, so this should always be false in multithreaded scenarios.
+  bool Enabled = false;
 };
 
 #define DEBUG_COUNTER(VARNAME, COUNTERNAME, DESC)                              \
diff --git a/llvm/lib/Support/DebugCounter.cpp b/llvm/lib/Support/DebugCounter.cpp
index 5a9cecfc56d4f..9c12de0776ad6 100644
--- a/llvm/lib/Support/DebugCounter.cpp
+++ b/llvm/lib/Support/DebugCounter.cpp
@@ -82,6 +82,7 @@ void DebugCounter::push_back(const std::string &Val) {
              << " is not a registered counter\n";
       return;
     }
+    enableAllCounters();
     Counters[CounterID].Skip = CounterVal;
     Counters[CounterID].IsSet = true;
   } else if (CounterPair.first.endswith("-count")) {
@@ -92,6 +93,7 @@ void DebugCounter::push_back(const std::string &Val) {
              << " is not a registered counter\n";
       return;
     }
+    enableAllCounters();
     Counters[CounterID].StopAfter = CounterVal;
     Counters[CounterID].IsSet = true;
   } else {