[Openmp-commits] [openmp] 88a68de - [OpenMP][NFCI] Split assertion message from assertion expression

Johannes Doerfert via Openmp-commits openmp-commits at lists.llvm.org
Tue Jul 18 16:57:22 PDT 2023


Author: Johannes Doerfert
Date: 2023-07-18T16:50:50-07:00
New Revision: 88a68de14cf66d067be54b8127c17b51192f2aa8

URL: https://github.com/llvm/llvm-project/commit/88a68de14cf66d067be54b8127c17b51192f2aa8
DIFF: https://github.com/llvm/llvm-project/commit/88a68de14cf66d067be54b8127c17b51192f2aa8.diff

LOG: [OpenMP][NFCI] Split assertion message from assertion expression

We ended up with `llvm.assume(icmp ne ptr as(4) null, as(4) @str)`
because the string in address space 4 was not known to be non-null.
There is no need to create these assumes.

Added: 
    

Modified: 
    openmp/libomptarget/DeviceRTL/include/Debug.h
    openmp/libomptarget/DeviceRTL/include/State.h
    openmp/libomptarget/DeviceRTL/src/Debug.cpp
    openmp/libomptarget/DeviceRTL/src/Kernel.cpp
    openmp/libomptarget/DeviceRTL/src/Mapping.cpp
    openmp/libomptarget/DeviceRTL/src/Parallelism.cpp
    openmp/libomptarget/DeviceRTL/src/State.cpp
    openmp/libomptarget/DeviceRTL/src/Synchronization.cpp

Removed: 
    


################################################################################
diff  --git a/openmp/libomptarget/DeviceRTL/include/Debug.h b/openmp/libomptarget/DeviceRTL/include/Debug.h
index 128572dfec606f..c7bcfcea57124a 100644
--- a/openmp/libomptarget/DeviceRTL/include/Debug.h
+++ b/openmp/libomptarget/DeviceRTL/include/Debug.h
@@ -20,14 +20,14 @@
 /// {
 extern "C" {
 void __assert_assume(bool condition);
-void __assert_fail(const char *assertion, const char *file, unsigned line,
-                   const char *function);
+void __assert_fail(const char *expr, const char *msg, const char *file,
+                   unsigned line, const char *function);
 }
 
-#define ASSERT(expr)                                                           \
+#define ASSERT(expr, msg)                                                      \
   {                                                                            \
     if (config::isDebugMode(config::DebugKind::Assertion) && !(expr))          \
-      __assert_fail(#expr, __FILE__, __LINE__, __PRETTY_FUNCTION__);           \
+      __assert_fail(#expr, msg, __FILE__, __LINE__, __PRETTY_FUNCTION__);      \
     else                                                                       \
       __assert_assume(expr);                                                   \
   }

diff  --git a/openmp/libomptarget/DeviceRTL/include/State.h b/openmp/libomptarget/DeviceRTL/include/State.h
index aac5a2275fcac1..c269cf1353c85a 100644
--- a/openmp/libomptarget/DeviceRTL/include/State.h
+++ b/openmp/libomptarget/DeviceRTL/include/State.h
@@ -153,7 +153,7 @@ inline uint32_t &lookupForModify32Impl(uint32_t state::ICVStateTy::*Var,
   if (OMP_UNLIKELY(!ThreadStates[TId])) {
     ThreadStates[TId] = reinterpret_cast<ThreadStateTy *>(memory::allocGlobal(
         sizeof(ThreadStateTy), "ICV modification outside data environment"));
-    ASSERT(ThreadStates[TId] != nullptr && "Nullptr returned by malloc!");
+    ASSERT(ThreadStates[TId] != nullptr, "Nullptr returned by malloc!");
     TeamState.HasThreadState = true;
     ThreadStates[TId]->init();
   }
@@ -248,7 +248,7 @@ template <typename Ty, ValueKind Kind> struct Value {
   __attribute__((flatten, always_inline)) void
   assert_eq(const Ty &V, IdentTy *Ident = nullptr,
             bool ForceTeamState = false) {
-    ASSERT(lookup(/* IsReadonly */ true, Ident, ForceTeamState) == V);
+    ASSERT(lookup(/* IsReadonly */ true, Ident, ForceTeamState) == V, nullptr);
   }
 
 private:
@@ -308,8 +308,7 @@ template <typename VTy, typename Ty> struct ValueRAII {
         Val(OldValue), Active(Active) {
     if (!Active)
       return;
-    ASSERT(*Ptr == OldValue &&
-           "ValueRAII initialization with wrong old value!");
+    ASSERT(*Ptr == OldValue, "ValueRAII initialization with wrong old value!");
     *Ptr = NewValue;
   }
   ~ValueRAII() {

diff  --git a/openmp/libomptarget/DeviceRTL/src/Debug.cpp b/openmp/libomptarget/DeviceRTL/src/Debug.cpp
index a1b289e830227b..2eaf3436e287c1 100644
--- a/openmp/libomptarget/DeviceRTL/src/Debug.cpp
+++ b/openmp/libomptarget/DeviceRTL/src/Debug.cpp
@@ -23,10 +23,14 @@ using namespace ompx;
 extern "C" {
 void __assert_assume(bool condition) { __builtin_assume(condition); }
 
-void __assert_fail(const char *assertion, const char *file, unsigned line,
-                   const char *function) {
-  PRINTF("%s:%u: %s: Assertion `%s' failed.\n", file, line, function,
-         assertion);
+void __assert_fail(const char *expr, const char *msg, const char *file,
+                   unsigned line, const char *function) {
+  if (msg) {
+    PRINTF("%s:%u: %s: Assertion %s (`%s') failed.\n", file, line, function,
+           msg, expr);
+  } else {
+    PRINTF("%s:%u: %s: Assertion `%s' failed.\n", file, line, function, expr);
+  }
   __builtin_trap();
 }
 }

diff  --git a/openmp/libomptarget/DeviceRTL/src/Kernel.cpp b/openmp/libomptarget/DeviceRTL/src/Kernel.cpp
index fa774afe469b7b..9502d1af6357b3 100644
--- a/openmp/libomptarget/DeviceRTL/src/Kernel.cpp
+++ b/openmp/libomptarget/DeviceRTL/src/Kernel.cpp
@@ -51,7 +51,7 @@ static void genericStateMachine(IdentTy *Ident) {
       return;
 
     if (IsActive) {
-      ASSERT(!mapping::isSPMDMode());
+      ASSERT(!mapping::isSPMDMode(), nullptr);
       ((void (*)(uint32_t, uint32_t))WorkFn)(0, TId);
       __kmpc_kernel_end_parallel();
     }
@@ -119,7 +119,7 @@ int32_t __kmpc_target_init(IdentTy *Ident, int8_t Mode,
     // ParallelRegionFn, which leads to bad results later on.
     ParallelRegionFnTy WorkFn = nullptr;
     __kmpc_kernel_parallel(&WorkFn);
-    ASSERT(WorkFn == nullptr);
+    ASSERT(WorkFn == nullptr, nullptr);
   }
 
   return mapping::getThreadIdInBlock();

diff  --git a/openmp/libomptarget/DeviceRTL/src/Mapping.cpp b/openmp/libomptarget/DeviceRTL/src/Mapping.cpp
index ad6897ec7bcf2d..447b3476ebcc68 100644
--- a/openmp/libomptarget/DeviceRTL/src/Mapping.cpp
+++ b/openmp/libomptarget/DeviceRTL/src/Mapping.cpp
@@ -199,13 +199,13 @@ LaneMaskTy mapping::lanemaskGT() { return impl::lanemaskGT(); }
 
 uint32_t mapping::getThreadIdInWarp() {
   uint32_t ThreadIdInWarp = impl::getThreadIdInWarp();
-  ASSERT(ThreadIdInWarp < impl::getWarpSize());
+  ASSERT(ThreadIdInWarp < impl::getWarpSize(), nullptr);
   return ThreadIdInWarp;
 }
 
 uint32_t mapping::getThreadIdInBlock() {
   uint32_t ThreadIdInBlock = impl::getThreadIdInBlock();
-  ASSERT(ThreadIdInBlock < impl::getNumHardwareThreadsInBlock());
+  ASSERT(ThreadIdInBlock < impl::getNumHardwareThreadsInBlock(), nullptr);
   return ThreadIdInBlock;
 }
 
@@ -224,31 +224,31 @@ uint32_t mapping::getKernelSize() { return impl::getKernelSize(); }
 
 uint32_t mapping::getWarpId() {
   uint32_t WarpID = impl::getWarpId();
-  ASSERT(WarpID < impl::getNumberOfWarpsInBlock());
+  ASSERT(WarpID < impl::getNumberOfWarpsInBlock(), nullptr);
   return WarpID;
 }
 
 uint32_t mapping::getBlockId() {
   uint32_t BlockId = impl::getBlockId();
-  ASSERT(BlockId < impl::getNumberOfBlocks());
+  ASSERT(BlockId < impl::getNumberOfBlocks(), nullptr);
   return BlockId;
 }
 
 uint32_t mapping::getNumberOfWarpsInBlock() {
   uint32_t NumberOfWarpsInBlocks = impl::getNumberOfWarpsInBlock();
-  ASSERT(impl::getWarpId() < NumberOfWarpsInBlocks);
+  ASSERT(impl::getWarpId() < NumberOfWarpsInBlocks, nullptr);
   return NumberOfWarpsInBlocks;
 }
 
 uint32_t mapping::getNumberOfBlocks() {
   uint32_t NumberOfBlocks = impl::getNumberOfBlocks();
-  ASSERT(impl::getBlockId() < NumberOfBlocks);
+  ASSERT(impl::getBlockId() < NumberOfBlocks, nullptr);
   return NumberOfBlocks;
 }
 
 uint32_t mapping::getNumberOfProcessorElements() {
   uint32_t NumberOfProcessorElements = impl::getNumHardwareThreadsInBlock();
-  ASSERT(impl::getThreadIdInBlock() < NumberOfProcessorElements);
+  ASSERT(impl::getThreadIdInBlock() < NumberOfProcessorElements, nullptr);
   return NumberOfProcessorElements;
 }
 

diff  --git a/openmp/libomptarget/DeviceRTL/src/Parallelism.cpp b/openmp/libomptarget/DeviceRTL/src/Parallelism.cpp
index d32dd7e4f99806..78cd6c046f9c89 100644
--- a/openmp/libomptarget/DeviceRTL/src/Parallelism.cpp
+++ b/openmp/libomptarget/DeviceRTL/src/Parallelism.cpp
@@ -91,7 +91,7 @@ void __kmpc_parallel_51(IdentTy *ident, int32_t, int32_t if_expr,
   uint32_t TId = mapping::getThreadIdInBlock();
 
   // Assert the parallelism level is zero if disabled by the user.
-  ASSERT((config::mayUseNestedParallelism() || icv::Level == 0) &&
+  ASSERT((config::mayUseNestedParallelism() || icv::Level == 0),
          "nested parallelism while disabled");
 
   // Handle the serialized case first, same for SPMD/non-SPMD:
@@ -107,7 +107,7 @@ void __kmpc_parallel_51(IdentTy *ident, int32_t, int32_t if_expr,
   }
 
   // From this point forward we know that there is no thread state used.
-  ASSERT(state::HasThreadState == false);
+  ASSERT(state::HasThreadState == false, nullptr);
 
   uint32_t NumThreads = determineNumberOfThreads(num_threads);
   if (mapping::isSPMDMode()) {
@@ -280,10 +280,10 @@ __attribute__((noinline)) void __kmpc_kernel_end_parallel() {
   FunctionTracingRAII();
   // In case we have modified an ICV for this thread before a ThreadState was
   // created. We drop it now to not contaminate the next parallel region.
-  ASSERT(!mapping::isSPMDMode());
+  ASSERT(!mapping::isSPMDMode(), nullptr);
   uint32_t TId = mapping::getThreadIdInBlock();
   state::resetStateForThread(TId);
-  ASSERT(!mapping::isSPMDMode());
+  ASSERT(!mapping::isSPMDMode(), nullptr);
 }
 
 uint16_t __kmpc_parallel_level(IdentTy *, uint32_t) {

diff  --git a/openmp/libomptarget/DeviceRTL/src/State.cpp b/openmp/libomptarget/DeviceRTL/src/State.cpp
index 9c1c9abaf493e9..b97d230fd50194 100644
--- a/openmp/libomptarget/DeviceRTL/src/State.cpp
+++ b/openmp/libomptarget/DeviceRTL/src/State.cpp
@@ -142,7 +142,7 @@ void *SharedMemorySmartStackTy::push(uint64_t Bytes) {
   void *GlobalMemory = memory::allocGlobal(
       AlignedBytes, "Slow path shared memory allocation, insufficient "
                     "shared memory stack memory!");
-  ASSERT(GlobalMemory != nullptr && "nullptr returned by malloc!");
+  ASSERT(GlobalMemory != nullptr, "nullptr returned by malloc!");
 
   return GlobalMemory;
 }
@@ -189,12 +189,12 @@ bool state::ICVStateTy::operator==(const ICVStateTy &Other) const {
 }
 
 void state::ICVStateTy::assertEqual(const ICVStateTy &Other) const {
-  ASSERT(NThreadsVar == Other.NThreadsVar);
-  ASSERT(LevelVar == Other.LevelVar);
-  ASSERT(ActiveLevelVar == Other.ActiveLevelVar);
-  ASSERT(MaxActiveLevelsVar == Other.MaxActiveLevelsVar);
-  ASSERT(RunSchedVar == Other.RunSchedVar);
-  ASSERT(RunSchedChunkVar == Other.RunSchedChunkVar);
+  ASSERT(NThreadsVar == Other.NThreadsVar, nullptr);
+  ASSERT(LevelVar == Other.LevelVar, nullptr);
+  ASSERT(ActiveLevelVar == Other.ActiveLevelVar, nullptr);
+  ASSERT(MaxActiveLevelsVar == Other.MaxActiveLevelsVar, nullptr);
+  ASSERT(RunSchedVar == Other.RunSchedVar, nullptr);
+  ASSERT(RunSchedChunkVar == Other.RunSchedChunkVar, nullptr);
 }
 
 void state::TeamStateTy::init(bool IsSPMD) {
@@ -217,8 +217,8 @@ bool state::TeamStateTy::operator==(const TeamStateTy &Other) const {
 
 void state::TeamStateTy::assertEqual(TeamStateTy &Other) const {
   ICVState.assertEqual(Other.ICVState);
-  ASSERT(ParallelTeamSize == Other.ParallelTeamSize);
-  ASSERT(HasThreadState == Other.HasThreadState);
+  ASSERT(ParallelTeamSize == Other.ParallelTeamSize, nullptr);
+  ASSERT(HasThreadState == Other.HasThreadState, nullptr);
 }
 
 state::TeamStateTy SHARED(ompx::state::TeamState);
@@ -251,7 +251,7 @@ void state::init(bool IsSPMD) {
 }
 
 void state::enterDataEnvironment(IdentTy *Ident) {
-  ASSERT(config::mayUseThreadStates() &&
+  ASSERT(config::mayUseThreadStates(),
          "Thread state modified while explicitly disabled!");
   if (!config::mayUseThreadStates())
     return;
@@ -269,7 +269,7 @@ void state::enterDataEnvironment(IdentTy *Ident) {
                      atomic::seq_cst, atomic::seq_cst))
       memory::freeGlobal(ThreadStatesPtr,
                          "Thread state array allocated multiple times");
-    ASSERT(atomic::load(ThreadStatesBitsPtr, atomic::seq_cst) &&
+    ASSERT(atomic::load(ThreadStatesBitsPtr, atomic::seq_cst),
            "Expected valid thread states bit!");
   }
   NewThreadState->init(ThreadStates[TId]);
@@ -278,7 +278,7 @@ void state::enterDataEnvironment(IdentTy *Ident) {
 }
 
 void state::exitDataEnvironment() {
-  ASSERT(config::mayUseThreadStates() &&
+  ASSERT(config::mayUseThreadStates(),
          "Thread state modified while explicitly disabled!");
 
   unsigned TId = mapping::getThreadIdInBlock();
@@ -309,7 +309,7 @@ void state::assumeInitialState(bool IsSPMD) {
   TeamStateTy InitialTeamState;
   InitialTeamState.init(IsSPMD);
   InitialTeamState.assertEqual(TeamState);
-  ASSERT(mapping::isSPMDMode() == IsSPMD);
+  ASSERT(mapping::isSPMDMode() == IsSPMD, nullptr);
 }
 
 extern "C" {
@@ -323,7 +323,7 @@ int omp_get_max_threads(void) { return icv::NThreads; }
 
 int omp_get_level(void) {
   int LevelVar = icv::Level;
-  ASSERT(LevelVar >= 0);
+  ASSERT(LevelVar >= 0, nullptr);
   return LevelVar;
 }
 
@@ -445,7 +445,7 @@ void __kmpc_begin_sharing_variables(void ***GlobalArgs, uint64_t nArgs) {
   } else {
     SharedMemVariableSharingSpacePtr = (void **)memory::allocGlobal(
         nArgs * sizeof(void *), "new extended args");
-    ASSERT(SharedMemVariableSharingSpacePtr != nullptr &&
+    ASSERT(SharedMemVariableSharingSpacePtr != nullptr,
            "Nullptr returned by malloc!");
   }
   *GlobalArgs = SharedMemVariableSharingSpacePtr;

diff  --git a/openmp/libomptarget/DeviceRTL/src/Synchronization.cpp b/openmp/libomptarget/DeviceRTL/src/Synchronization.cpp
index 550d9614a963e8..36536b7a81a164 100644
--- a/openmp/libomptarget/DeviceRTL/src/Synchronization.cpp
+++ b/openmp/libomptarget/DeviceRTL/src/Synchronization.cpp
@@ -331,7 +331,7 @@ void namedBarrierInit() {}
 
 void namedBarrier() {
   uint32_t NumThreads = omp_get_num_threads();
-  ASSERT(NumThreads % 32 == 0);
+  ASSERT(NumThreads % 32 == 0, nullptr);
 
   // The named barrier for active parallel threads of a team in an L1 parallel
   // region to synchronize with each other.


        


More information about the Openmp-commits mailing list