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

[Rafael Espíndola via llvm-commits]

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