[OpenMP] Ensure broken assumptions print once, not thousands of times.

[jdoerfert]

Summary

If we have a broken assumption we want to print a message to the user.
If the assumption is broken by many threads in many teams this can
become a problem. To avoid it we use a hash that tracks if a broken
assumption has (likely) been printed and avoid printing it again. This
is not fool proof and has some caveats that might cause problems in
the future (see comment) but it should improve the situation
considerably for now.

[tianshilei1992]

openmp/libomptarget/DeviceRTL/src/Debug.cpp

43 ↗

(On Diff #381008)

Seems better to wrap it with a function or macro like PRINTF_ONCE.

[jhuber6]

openmp/libomptarget/DeviceRTL/src/Debug.cpp

44 ↗

(On Diff #381008)

Unrelated, but why don't we use the standard format from assert.h along with using PRETTY_FUNCTION?

[jdoerfert]

Use singleton flag per assumption

[jdoerfert]

openmp/libomptarget/DeviceRTL/src/Debug.cpp

44 ↗

(On Diff #381008)

Idk, let's do that separate.

[JonChesterfield]

openmp/libomptarget/DeviceRTL/include/Utils.h

80

Are we c++17 in the deviceRTL? Can presumably be whatever clang trunk has implemented

99

Or we could use zero for the initial value and write 1 into it. Seq_cst seems unnecessary - acq_rel? Global ordering on GPUs is expensive and not obviously required here, don't care which thread wins the race to print a debug message

openmp/libomptarget/DeviceRTL/src/Synchronization.cpp

324

Normally takes two orderings

[JonChesterfield]

openmp/libomptarget/DeviceRTL/include/Utils.h

99

Actually worse than that - seq cst induces a global order. To make that work, we probably change thread execution ordering. Want to minimise assert's influence on dynamic control flow

[jdoerfert]

openmp/libomptarget/DeviceRTL/include/Utils.h

80

Runs w/o warnings for me. I'll use an integer type.

99

Or we could use zero for the initial value and write 1 into it.

We could, but then we would pay a cost even if this code is dead. I'm fine either way.

...

This is used in front of a __builtin_trap, or at least guarded by a debug/configuration kind. It will not be fast but it doesn't have to be.

openmp/libomptarget/DeviceRTL/src/Synchronization.cpp

324

No it does not. This is not cas.

[jdoerfert]

Remove UB, use weaker atomics

[JonChesterfield]

openmp/libomptarget/DeviceRTL/src/Synchronization.cpp

324

Yep, thanks! Did indeed misread as compare&swap

[jhuber6]

Rebasing

[jhuber6]

Fix leftover code

[JonChesterfield]

Counter is per-tu and this is defined and used in a header. What happens if it's used in two translation units? Instantiations collide, linker picks one, we miss an output?

[jdoerfert]

In D112156#3277368, @JonChesterfield wrote:

Counter is per-tu and this is defined and used in a header. What happens if it's used in two translation units? Instantiations collide, linker picks one, we miss an output?

We can replace counter with an enum if we want more control.

[JonChesterfield]

Not sure I care hugely about control, but if these are colliding between TUs we lose quite a lot of precision in the reporting. Wrapping the function in an anonymous namespace would be a fix

[jhuber6]

Replacing COUNTER with a defined enum value to uniquely identify which singleton counter to query between TUs.

[jhuber6]

Changing to use namespace and counter again.

openmp/libomptarget/DeviceRTL/include/Debug.h

 //===-------- Debug.h ---- Debug utilities ------------------------ C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef OMPTARGET_DEVICERTL_DEBUG_H
 #define OMPTARGET_DEVICERTL_DEBUG_H
 
 #include "Configuration.h"
+#include "Utils.h"
 
 /// Assertion
 ///
 /// {
 extern "C" {
 void __assert_assume(bool condition);
 void __assert_fail(const char *assertion, const char *file, unsigned line,
                    const char *function);
 }
 
 #define ASSERT(expr)                                                           \
   {                                                                            \
-    if (config::isDebugMode(config::DebugKind::Assertion) && !(expr))          \
+    if (config::isDebugMode(config::DebugKind::Assertion) && !(expr) &&        \
+        utils::SingletonFlag::testAndSet())                                    \
       __assert_fail(#expr, __FILE__, __LINE__, __PRETTY_FUNCTION__);           \
-    else                                                                       \
+    else if (!config::isDebugMode(config::DebugKind::Assertion))               \
       __assert_assume(expr);                                                   \
   }
 
 ///}
 
 /// Print
 /// printf() calls are rewritten by CGGPUBuiltin to __llvm_omp_vprintf
 /// {

openmp/libomptarget/DeviceRTL/include/Synchronization.h

 uint32_t inc(uint32_t *Addr, uint32_t V, int Ordering);
 
 /// Atomically add \p V to \p *Addr with \p Ordering semantics.
 uint32_t add(uint32_t *Addr, uint32_t V, int Ordering);
 
 /// Atomically add \p V to \p *Addr with \p Ordering semantics.
 uint64_t add(uint64_t *Addr, uint64_t V, int Ordering);
 
+/// Atomically write \p V to \p *Addr with \p Ordering semantics and return the
+/// old value of \p *Addr.
+uint32_t exchange(uint32_t *Addr, uint32_t V, int Ordering);
+
 } // namespace atomic
 
 } // namespace _OMP
 
 #endif

openmp/libomptarget/DeviceRTL/include/Utils.h

 //===--------- Utils.h - OpenMP device runtime utility functions -- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef OMPTARGET_DEVICERTL_UTILS_H
 #define OMPTARGET_DEVICERTL_UTILS_H
 
+#include "Synchronization.h"
 #include "Types.h"
 
 namespace _OMP {
 namespace utils {
 
 /// Return the value \p Var from thread Id \p SrcLane in the warp if the thread
 /// is identified by \p Mask.
 int32_t shuffle(uint64_t Mask, int32_t Var, int32_t SrcLane);

 template <typename Ty1, typename Ty2> inline Ty1 align_up(Ty1 V, Ty2 Align) {
   return ((V + Ty1(Align) - 1) / Ty1(Align)) * Ty1(Align);
 }
 /// Return \p V aligned "downwards" according to \p Align.
 template <typename Ty1, typename Ty2> inline Ty1 align_down(Ty1 V, Ty2 Align) {
   return V - V % Align;
 }
 
+namespace {
+/// Helper class to perform an action only once.
+///
+/// Using this is probably costly even if it is not executed. It should be
+/// guarded such that release mode execution will not be impacted.
+template <uint32_t ID> struct SingletonFlagImpl {
+
+  /// Each SingletonFlag instantiation with the same ID has an internal flag.
+  /// This function will return true if the flag was not set before, otherwise
+  /// it will return false. In either case the flag is set afterwards.
+  static bool testAndSet() {
+    static uint32_t DoOnceFlag = 0;
+    return 1 != atomic::exchange(&DoOnceFlag, 1, __ATOMIC_ACQ_REL);
+  }
+};
+} // namespace
+
+/// Helper to hide the __COUNTER__ use away.
+#define SingletonFlag SingletonFlagImpl<__COUNTER__>
+
 #define OMP_LIKELY(EXPR) __builtin_expect((bool)(EXPR), true)
 #define OMP_UNLIKELY(EXPR) __builtin_expect((bool)(EXPR), false)
 
 } // namespace utils
 } // namespace _OMP
 
 #endif

openmp/libomptarget/DeviceRTL/src/Synchronization.cpp

 uint32_t atomic::add(uint32_t *Addr, uint32_t V, int Ordering) {
   return impl::atomicAdd(Addr, V, Ordering);
 }
 
 uint64_t atomic::add(uint64_t *Addr, uint64_t V, int Ordering) {
   return impl::atomicAdd(Addr, V, Ordering);
 }
 
+uint32_t atomic::exchange(uint32_t *Addr, uint32_t V, int Ordering) {
+  return impl::atomicExchange(Addr, V, Ordering);
+}
+
 extern "C" {
 void __kmpc_ordered(IdentTy *Loc, int32_t TId) { FunctionTracingRAII(); }
 
 void __kmpc_end_ordered(IdentTy *Loc, int32_t TId) { FunctionTracingRAII(); }
 
 int32_t __kmpc_cancel_barrier(IdentTy *Loc, int32_t TId) {
   FunctionTracingRAII();
   __kmpc_barrier(Loc, TId);