From 88a68de14cf66d067be54b8127c17b51192f2aa8 Mon Sep 17 00:00:00 2001 From: Johannes Doerfert Date: Tue, 18 Jul 2023 16:05:08 -0700 Subject: [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. --- openmp/libomptarget/DeviceRTL/include/Debug.h | 8 +++--- openmp/libomptarget/DeviceRTL/include/State.h | 7 +++-- openmp/libomptarget/DeviceRTL/src/Debug.cpp | 12 ++++++--- openmp/libomptarget/DeviceRTL/src/Kernel.cpp | 4 +-- openmp/libomptarget/DeviceRTL/src/Mapping.cpp | 14 +++++----- openmp/libomptarget/DeviceRTL/src/Parallelism.cpp | 8 +++--- openmp/libomptarget/DeviceRTL/src/State.cpp | 30 +++++++++++----------- .../libomptarget/DeviceRTL/src/Synchronization.cpp | 2 +- 8 files changed, 44 insertions(+), 41 deletions(-) (limited to 'openmp') diff --git a/openmp/libomptarget/DeviceRTL/include/Debug.h b/openmp/libomptarget/DeviceRTL/include/Debug.h index 128572d..c7bcfce 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 aac5a22..c269cf1 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(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 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 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 a1b289e..2eaf343 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 fa774af..9502d1a 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 ad6897e..447b347 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 d32dd7e..78cd6c0 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 9c1c9ab..b97d230 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 550d961..36536b7 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. -- cgit v1.1