Merging r338762:

------------------------------------------------------------------------
r338762 | gbiv | 2018-08-02 21:50:27 +0200 (Thu, 02 Aug 2018) | 15 lines

[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

------------------------------------------------------------------------


git-svn-id: https://llvm.org/svn/llvm-project/llvm/branches/release_70@338846 91177308-0d34-0410-b5e6-96231b3b80d8

2 files changed, 25 insertions, 5 deletions

diff --git a/include/llvm/Support/DebugCounter.h b/include/llvm/Support/DebugCounter.h
index 250fc6bb1f5..83bd5a06c94 100644
--- a/include/llvm/Support/DebugCounter.h
+++ b/include/llvm/Support/DebugCounter.h
@@ -70,10 +70,9 @@ public:
     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 @@ public:
     }
     // 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 @@ public:
   }
   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 @@ private:
   };
   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/lib/Support/DebugCounter.cpp b/lib/Support/DebugCounter.cpp
index 5a9cecfc56d..9c12de0776a 100644
--- a/lib/Support/DebugCounter.cpp
+++ b/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 {