[llvm] r271504 - Switch statistics to use relaxed updates to a std::atomic.

[Chandler Carruth via llvm-commits]

Yep. I expect to succeed at that in about two more patches.

On Thu, Jun 2, 2016, 16:11 Rafael Espíndola <llvm-commits at lists.llvm.org>
wrote:

> Cool, cas_flag is almost dead now. Looks like we can remove it once
> std::call_once works everywhere.
>
> Cheers,
> Rafael
>
>
> On 2 June 2016 at 01:44, Chandler Carruth via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
> > Author: chandlerc
> > Date: Thu Jun  2 03:44:05 2016
> > New Revision: 271504
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=271504&view=rev
> > Log:
> > Switch statistics to use relaxed updates to a std::atomic.
> >
> > This removes usage of the hacky, incorrect, and TSan-unfriendly
> > home-grown atomics. It should actually be more efficient in some cases.
> >
> > Based on our existing usage of <atomic>, all of this is portably
> > available AFAICT. One small challenge is initializing the stastic, but
> > I've tried a comparable sample out on MSVC (the most likely to complain
> > here) and it seems to work. Will have to watch the build bots of course.
> >
> > Modified:
> >     llvm/trunk/include/llvm/ADT/Statistic.h
> >
> > Modified: llvm/trunk/include/llvm/ADT/Statistic.h
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/Statistic.h?rev=271504&r1=271503&r2=271504&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/include/llvm/ADT/Statistic.h (original)
> > +++ llvm/trunk/include/llvm/ADT/Statistic.h Thu Jun  2 03:44:05 2016
> > @@ -28,6 +28,7 @@
> >
> >  #include "llvm/Support/Atomic.h"
> >  #include "llvm/Support/Compiler.h"
> > +#include <atomic>
> >  #include <memory>
> >
> >  namespace llvm {
> > @@ -38,10 +39,10 @@ class Statistic {
> >  public:
> >    const char *Name;
> >    const char *Desc;
> > -  volatile llvm::sys::cas_flag Value;
> > +  std::atomic<unsigned> Value;
> >    bool Initialized;
> >
> > -  llvm::sys::cas_flag getValue() const { return Value; }
> > +  unsigned getValue() const { return
> Value.load(std::memory_order_relaxed); }
> >    const char *getName() const { return Name; }
> >    const char *getDesc() const { return Desc; }
> >
> > @@ -52,51 +53,45 @@ public:
> >    }
> >
> >    // Allow use of this class as the value itself.
> > -  operator unsigned() const { return Value; }
> > +  operator unsigned() const { return getValue(); }
> >
> >  #if !defined(NDEBUG) || defined(LLVM_ENABLE_STATS)
> >     const Statistic &operator=(unsigned Val) {
> > -    Value = Val;
> > +    Value.store(Val, std::memory_order_relaxed);
> >      return init();
> >    }
> >
> >    const Statistic &operator++() {
> > -    // FIXME: This function and all those that follow carefully use an
> > -    // atomic operation to update the value safely in the presence of
> > -    // concurrent accesses, but not to read the return value, so the
> > -    // return value is not thread safe.
> > -    sys::AtomicIncrement(&Value);
> > +    Value.fetch_add(1, std::memory_order_relaxed);
> >      return init();
> >    }
> >
> >    unsigned operator++(int) {
> >      init();
> > -    unsigned OldValue = Value;
> > -    sys::AtomicIncrement(&Value);
> > -    return OldValue;
> > +    return Value.fetch_add(1, std::memory_order_relaxed);
> >    }
> >
> >    const Statistic &operator--() {
> > -    sys::AtomicDecrement(&Value);
> > +    Value.fetch_sub(1, std::memory_order_relaxed);
> >      return init();
> >    }
> >
> >    unsigned operator--(int) {
> >      init();
> > -    unsigned OldValue = Value;
> > -    sys::AtomicDecrement(&Value);
> > -    return OldValue;
> > +    return Value.fetch_sub(1, std::memory_order_relaxed);
> >    }
> >
> > -  const Statistic &operator+=(const unsigned &V) {
> > -    if (!V) return *this;
> > -    sys::AtomicAdd(&Value, V);
> > +  const Statistic &operator+=(unsigned V) {
> > +    if (V == 0)
> > +      return *this;
> > +    Value.fetch_add(V, std::memory_order_relaxed);
> >      return init();
> >    }
> >
> > -  const Statistic &operator-=(const unsigned &V) {
> > -    if (!V) return *this;
> > -    sys::AtomicAdd(&Value, -V);
> > +  const Statistic &operator-=(unsigned V) {
> > +    if (V == 0)
> > +      return *this;
> > +    Value.fetch_sub(V, std::memory_order_relaxed);
> >      return init();
> >    }
> >
> > @@ -145,8 +140,8 @@ protected:
> >
> >  // STATISTIC - A macro to make definition of statistics really simple.
> This
> >  // automatically passes the DEBUG_TYPE of the file into the statistic.
> > -#define STATISTIC(VARNAME, DESC) \
> > -  static llvm::Statistic VARNAME = { DEBUG_TYPE, DESC, 0, 0 }
> > +#define STATISTIC(VARNAME, DESC)
>        \
> > +  static llvm::Statistic VARNAME = {DEBUG_TYPE, DESC, {0}, 0}
> >
> >  /// \brief Enable the collection and printing of statistics.
> >  void EnableStatistics();
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160603/0c5e0caf/attachment.html>