[llvm] r338762 - [Support] Add an enable bit to our DebugCounters

[Hans Wennborg via llvm-commits]

Merged in r338846. Thanks!

On Fri, Aug 3, 2018 at 3:18 AM, George Burgess IV
<george.burgess.iv at gmail.com> wrote:
> Hi!
>
> I'd like to get this merged into the 7.0 branch. It fixes data races, which
> were introduced by r337748, in asserting builds of LLVM.
>
> I've included Chandler as owner of Support/.
>
> Alternatively, we can revert r337748 on 7.0, since it's not all that useful
> on its own. :) I'm equally happy with either approach.
>
> Thank you!
> George
>
>
> On Thu, Aug 2, 2018 at 12:50 PM George Burgess IV via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
>>
>> Author: gbiv
>> Date: Thu Aug  2 12:50:27 2018
>> New Revision: 338762
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=338762&view=rev
>> Log:
>> [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
>>
>> Modified:
>>     llvm/trunk/include/llvm/Support/DebugCounter.h
>>     llvm/trunk/lib/Support/DebugCounter.cpp
>>
>> Modified: llvm/trunk/include/llvm/Support/DebugCounter.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/DebugCounter.h?rev=338762&r1=338761&r2=338762&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/include/llvm/Support/DebugCounter.h (original)
>> +++ llvm/trunk/include/llvm/Support/DebugCounter.h Thu Aug  2 12:50:27
>> 2018
>> @@ -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)
>> \
>>
>> Modified: llvm/trunk/lib/Support/DebugCounter.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/DebugCounter.cpp?rev=338762&r1=338761&r2=338762&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Support/DebugCounter.cpp (original)
>> +++ llvm/trunk/lib/Support/DebugCounter.cpp Thu Aug  2 12:50:27 2018
>> @@ -82,6 +82,7 @@ void DebugCounter::push_back(const std::
>>               << " 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::
>>               << " is not a registered counter\n";
>>        return;
>>      }
>> +    enableAllCounters();
>>      Counters[CounterID].StopAfter = CounterVal;
>>      Counters[CounterID].IsSet = true;
>>    } else {
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits