[Libomptarget] Address existing warnings in the device runtime library
authorJoseph Huber <jhuber6@vols.utk.edu>
Tue, 10 May 2022 21:33:41 +0000 (17:33 -0400)
committerJoseph Huber <jhuber6@vols.utk.edu>
Fri, 13 May 2022 18:38:31 +0000 (14:38 -0400)
This patche attemps to address the current warnings in the OpenMP
offloading device runtime. Previously we did not see these because we
compiled the runtime without the standard warning flags enabled.
However, these warnings are used when we now build the static library
version of this runtime. This became extremely noisy when coupled with
the fact the we compile each file roughly 32 times when all the
architectures are considered. So it would be ideal to not have all these
warnings show up when building.

Most of these errors were simply implicit switch-case fallthroughs,
which can be addressed using C++17's fallthrough attribute. Additionally
there was a volatile variable that was being casted away. This is most
likely safe to remove because we cast it away before its even used and
didn't seem to affect anything in testing.

Depends on D125260

Reviewed By: jdoerfert

Differential Revision: https://reviews.llvm.org/D125339

openmp/libomptarget/DeviceRTL/src/Debug.cpp
openmp/libomptarget/DeviceRTL/src/Mapping.cpp
openmp/libomptarget/DeviceRTL/src/Parallelism.cpp
openmp/libomptarget/DeviceRTL/src/Reduction.cpp
openmp/libomptarget/DeviceRTL/src/State.cpp
openmp/libomptarget/DeviceRTL/src/Workshare.cpp

index e97c77d..45e08fa 100644 (file)
@@ -38,7 +38,7 @@ int32_t omp_vprintf(const char *Format, void *Arguments, uint32_t);
     device = {arch(nvptx, nvptx64)}, implementation = {extension(match_any)})
 int32_t vprintf(const char *, void *);
 namespace impl {
-static int32_t omp_vprintf(const char *Format, void *Arguments, uint32_t) {
+int32_t omp_vprintf(const char *Format, void *Arguments, uint32_t) {
   return vprintf(Format, Arguments);
 }
 } // namespace impl
@@ -47,7 +47,7 @@ static int32_t omp_vprintf(const char *Format, void *Arguments, uint32_t) {
 // We do not have a vprintf implementation for AMD GPU yet so we use a stub.
 #pragma omp begin declare variant match(device = {arch(amdgcn)})
 namespace impl {
-static int32_t omp_vprintf(const char *Format, void *Arguments, uint32_t) {
+int32_t omp_vprintf(const char *Format, void *Arguments, uint32_t) {
   return -1;
 }
 } // namespace impl
index 48ca13a..12ef58f 100644 (file)
@@ -46,7 +46,7 @@ uint32_t getNumberOfWarpsInBlock();
 ///{
 #pragma omp begin declare variant match(device = {arch(amdgcn)})
 
-static const llvm::omp::GV &getGridValue() {
+const llvm::omp::GV &getGridValue() {
   return llvm::omp::getAMDGPUGridValues<__AMDGCN_WAVEFRONT_SIZE>();
 }
 
@@ -121,9 +121,7 @@ uint32_t getNumHardwareThreadsInBlock() {
   return __nvvm_read_ptx_sreg_ntid_x();
 }
 
-static const llvm::omp::GV &getGridValue() {
-  return llvm::omp::NVPTXGridValues;
-}
+const llvm::omp::GV &getGridValue() { return llvm::omp::NVPTXGridValues; }
 
 LaneMaskTy activemask() {
   unsigned int Mask;
index fd419b8..a4c73bd 100644 (file)
@@ -158,52 +158,52 @@ void __kmpc_parallel_51(IdentTy *ident, int32_t, int32_t if_expr,
       break;
     case 16:
       GlobalArgs[15] = args[15];
-      // FALLTHROUGH
+      [[fallthrough]];
     case 15:
       GlobalArgs[14] = args[14];
-      // FALLTHROUGH
+      [[fallthrough]];
     case 14:
       GlobalArgs[13] = args[13];
-      // FALLTHROUGH
+      [[fallthrough]];
     case 13:
       GlobalArgs[12] = args[12];
-      // FALLTHROUGH
+      [[fallthrough]];
     case 12:
       GlobalArgs[11] = args[11];
-      // FALLTHROUGH
+      [[fallthrough]];
     case 11:
       GlobalArgs[10] = args[10];
-      // FALLTHROUGH
+      [[fallthrough]];
     case 10:
       GlobalArgs[9] = args[9];
-      // FALLTHROUGH
+      [[fallthrough]];
     case 9:
       GlobalArgs[8] = args[8];
-      // FALLTHROUGH
+      [[fallthrough]];
     case 8:
       GlobalArgs[7] = args[7];
-      // FALLTHROUGH
+      [[fallthrough]];
     case 7:
       GlobalArgs[6] = args[6];
-      // FALLTHROUGH
+      [[fallthrough]];
     case 6:
       GlobalArgs[5] = args[5];
-      // FALLTHROUGH
+      [[fallthrough]];
     case 5:
       GlobalArgs[4] = args[4];
-      // FALLTHROUGH
+      [[fallthrough]];
     case 4:
       GlobalArgs[3] = args[3];
-      // FALLTHROUGH
+      [[fallthrough]];
     case 3:
       GlobalArgs[2] = args[2];
-      // FALLTHROUGH
+      [[fallthrough]];
     case 2:
       GlobalArgs[1] = args[1];
-      // FALLTHROUGH
+      [[fallthrough]];
     case 1:
       GlobalArgs[0] = args[0];
-      // FALLTHROUGH
+      [[fallthrough]];
     case 0:
       break;
     }
index 516da6b..e65cdd3 100644 (file)
@@ -167,8 +167,8 @@ uint32_t roundToWarpsize(uint32_t s) {
 
 uint32_t kmpcMin(uint32_t x, uint32_t y) { return x < y ? x : y; }
 
-static volatile uint32_t IterCnt = 0;
-static volatile uint32_t Cnt = 0;
+static uint32_t IterCnt = 0;
+static uint32_t Cnt = 0;
 
 } // namespace
 
@@ -211,7 +211,7 @@ int32_t __kmpc_nvptx_teams_reduce_nowait_v2(
   // to the number of slots in the buffer.
   bool IsMaster = (ThreadId == 0);
   while (IsMaster) {
-    Bound = atomic::load((uint32_t *)&IterCnt, __ATOMIC_SEQ_CST);
+    Bound = atomic::load(&IterCnt, __ATOMIC_SEQ_CST);
     if (TeamId < Bound + num_of_records)
       break;
   }
@@ -228,8 +228,7 @@ int32_t __kmpc_nvptx_teams_reduce_nowait_v2(
     // Increment team counter.
     // This counter is incremented by all teams in the current
     // BUFFER_SIZE chunk.
-    ChunkTeamCount =
-        atomic::inc((uint32_t *)&Cnt, num_of_records - 1u, __ATOMIC_SEQ_CST);
+    ChunkTeamCount = atomic::inc(&Cnt, num_of_records - 1u, __ATOMIC_SEQ_CST);
   }
   // Synchronize
   if (mapping::isSPMDMode())
@@ -305,8 +304,7 @@ int32_t __kmpc_nvptx_teams_reduce_nowait_v2(
   if (IsMaster && ChunkTeamCount == num_of_records - 1) {
     // Allow SIZE number of teams to proceed writing their
     // intermediate results to the global buffer.
-    atomic::add((uint32_t *)&IterCnt, uint32_t(num_of_records),
-                __ATOMIC_SEQ_CST);
+    atomic::add(&IterCnt, uint32_t(num_of_records), __ATOMIC_SEQ_CST);
   }
 
   return 0;
index 685c697..f299aa0 100644 (file)
@@ -298,14 +298,8 @@ uint32_t &lookupForModify32Impl(uint32_t ICVStateTy::*Var, IdentTy *Ident) {
   return ThreadStates[TId]->ICVState.*Var;
 }
 
-uint32_t &lookup32Impl(uint32_t ICVStateTy::*Var) {
-  uint32_t TId = mapping::getThreadIdInBlock();
-  if (OMP_UNLIKELY(config::mayUseThreadStates() && ThreadStates[TId]))
-    return ThreadStates[TId]->ICVState.*Var;
-  return TeamState.ICVState.*Var;
-}
-uint64_t &lookup64Impl(uint64_t ICVStateTy::*Var) {
-  uint64_t TId = mapping::getThreadIdInBlock();
+template <typename IntTy> IntTy &lookupImpl(IntTy ICVStateTy::*Var) {
+  IntTy TId = mapping::getThreadIdInBlock();
   if (OMP_UNLIKELY(config::mayUseThreadStates() && ThreadStates[TId]))
     return ThreadStates[TId]->ICVState.*Var;
   return TeamState.ICVState.*Var;
@@ -330,27 +324,27 @@ uint32_t &state::lookup32(ValueKind Kind, bool IsReadonly, IdentTy *Ident) {
   switch (Kind) {
   case state::VK_NThreads:
     if (IsReadonly)
-      return lookup32Impl(&ICVStateTy::NThreadsVar);
+      return lookupImpl<uint32_t>(&ICVStateTy::NThreadsVar);
     return lookupForModify32Impl(&ICVStateTy::NThreadsVar, Ident);
   case state::VK_Level:
     if (IsReadonly)
-      return lookup32Impl(&ICVStateTy::LevelVar);
+      return lookupImpl<uint32_t>(&ICVStateTy::LevelVar);
     return lookupForModify32Impl(&ICVStateTy::LevelVar, Ident);
   case state::VK_ActiveLevel:
     if (IsReadonly)
-      return lookup32Impl(&ICVStateTy::ActiveLevelVar);
+      return lookupImpl<uint32_t>(&ICVStateTy::ActiveLevelVar);
     return lookupForModify32Impl(&ICVStateTy::ActiveLevelVar, Ident);
   case state::VK_MaxActiveLevels:
     if (IsReadonly)
-      return lookup32Impl(&ICVStateTy::MaxActiveLevelsVar);
+      return lookupImpl<uint32_t>(&ICVStateTy::MaxActiveLevelsVar);
     return lookupForModify32Impl(&ICVStateTy::MaxActiveLevelsVar, Ident);
   case state::VK_RunSched:
     if (IsReadonly)
-      return lookup32Impl(&ICVStateTy::RunSchedVar);
+      return lookupImpl<uint32_t>(&ICVStateTy::RunSchedVar);
     return lookupForModify32Impl(&ICVStateTy::RunSchedVar, Ident);
   case state::VK_RunSchedChunk:
     if (IsReadonly)
-      return lookup32Impl(&ICVStateTy::RunSchedChunkVar);
+      return lookupImpl<uint32_t>(&ICVStateTy::RunSchedChunkVar);
     return lookupForModify32Impl(&ICVStateTy::RunSchedChunkVar, Ident);
   case state::VK_ParallelTeamSize:
     return TeamState.ParallelTeamSize;
index 81b3f6c..d168f21 100644 (file)
@@ -139,6 +139,7 @@ template <typename T, typename ST> struct omptarget_nvptx_LoopSupport {
                        numberOfActiveOMPThreads);
         break;
       }
+      [[fallthrough]];
     } // note: if chunk <=0, use nochunk
     case kmp_sched_static_balanced_chunk: {
       if (chunk > 0) {
@@ -157,6 +158,7 @@ template <typename T, typename ST> struct omptarget_nvptx_LoopSupport {
           ub = oldUb;
         break;
       }
+      [[fallthrough]];
     } // note: if chunk <=0, use nochunk
     case kmp_sched_static_nochunk: {
       ForStaticNoChunk(lastiter, lb, ub, stride, chunk, gtid,
@@ -168,8 +170,9 @@ template <typename T, typename ST> struct omptarget_nvptx_LoopSupport {
         ForStaticChunk(lastiter, lb, ub, stride, chunk, omp_get_team_num(),
                        omp_get_num_teams());
         break;
-      } // note: if chunk <=0, use nochunk
-    }
+      }
+      [[fallthrough]];
+    } // note: if chunk <=0, use nochunk
     case kmp_sched_distr_static_nochunk: {
       ForStaticNoChunk(lastiter, lb, ub, stride, chunk, omp_get_team_num(),
                        omp_get_num_teams());
@@ -341,7 +344,7 @@ template <typename T, typename ST> struct omptarget_nvptx_LoopSupport {
     uint32_t change = utils::popc(active);
     __kmpc_impl_lanemask_t lane_mask_lt = mapping::lanemaskLT();
     unsigned int rank = utils::popc(active & lane_mask_lt);
-    uint64_t warp_res;
+    uint64_t warp_res = 0;
     if (rank == 0) {
       warp_res = atomic::add(&Cnt, change, __ATOMIC_SEQ_CST);
     }