Skip to content

Commit

Permalink
[Support] Add an enable bit to our DebugCounters
Browse files Browse the repository at this point in the history
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
  • Loading branch information
gburgessiv committed Aug 2, 2018
1 parent dfd5fad commit 31da130
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 5 deletions.
28 changes: 23 additions & 5 deletions llvm/include/llvm/Support/DebugCounter.h
Expand Up @@ -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()) {
Expand All @@ -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
Expand Down Expand Up @@ -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] = {};
Expand All @@ -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) \
Expand Down
2 changes: 2 additions & 0 deletions llvm/lib/Support/DebugCounter.cpp
Expand Up @@ -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")) {
Expand All @@ -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 {
Expand Down

0 comments on commit 31da130

Please sign in to comment.