[Openmp-commits] [openmp] [OpenMP] Miscellaneous small code improvements (PR #95603)
Hansang Bae via Openmp-commits
openmp-commits at lists.llvm.org
Fri Jun 14 14:34:01 PDT 2024
https://github.com/hansangbae updated https://github.com/llvm/llvm-project/pull/95603
>From 03a9ccd1017af3abf68d1de41333e5484d10f366 Mon Sep 17 00:00:00 2001
From: Hansang Bae <hansang.bae at intel.com>
Date: Mon, 25 Sep 2023 14:52:03 -0500
Subject: [PATCH] [OpenMP] Miscellaneous small code improvements
Removes a few uninitialized variables, possible resource leaks, and
redundant code.
---
openmp/runtime/src/kmp.h | 2 ++
openmp/runtime/src/kmp_affinity.cpp | 6 +++---
openmp/runtime/src/kmp_affinity.h | 8 +++++---
openmp/runtime/src/kmp_barrier.cpp | 3 ++-
openmp/runtime/src/kmp_csupport.cpp | 6 +++---
openmp/runtime/src/kmp_runtime.cpp | 12 ++++++------
openmp/runtime/src/kmp_tasking.cpp | 2 +-
openmp/runtime/src/kmp_wait_release.h | 10 ++++++----
openmp/runtime/src/ompt-general.cpp | 6 ++++++
9 files changed, 34 insertions(+), 21 deletions(-)
diff --git a/openmp/runtime/src/kmp.h b/openmp/runtime/src/kmp.h
index f625e84a93d43..41694872fa945 100644
--- a/openmp/runtime/src/kmp.h
+++ b/openmp/runtime/src/kmp.h
@@ -4705,6 +4705,8 @@ class kmp_safe_raii_file_t {
: f(nullptr) {
open(filename, mode, env_var);
}
+ kmp_safe_raii_file_t(const kmp_safe_raii_file_t &other) = delete;
+ kmp_safe_raii_file_t &operator=(const kmp_safe_raii_file_t &other) = delete;
~kmp_safe_raii_file_t() { close(); }
/// Open filename using mode. This is automatically closed in the destructor.
diff --git a/openmp/runtime/src/kmp_affinity.cpp b/openmp/runtime/src/kmp_affinity.cpp
index f34e55555545f..9db043e5d5e47 100644
--- a/openmp/runtime/src/kmp_affinity.cpp
+++ b/openmp/runtime/src/kmp_affinity.cpp
@@ -1944,7 +1944,6 @@ static bool __kmp_affinity_create_hwloc_map(kmp_i18n_id_t *const msg_id) {
hw_thread.ids[index + 1] = sub_id;
index--;
}
- prev = memory;
}
prev = obj;
}
@@ -2126,7 +2125,8 @@ static int __kmp_affinity_cmp_apicThreadInfo_phys_id(const void *a,
class kmp_cache_info_t {
public:
struct info_t {
- unsigned level, mask;
+ unsigned level = 0;
+ unsigned mask = 0;
};
kmp_cache_info_t() : depth(0) { get_leaf4_levels(); }
size_t get_depth() const { return depth; }
@@ -4715,7 +4715,7 @@ static void __kmp_aux_affinity_initialize(kmp_affinity_t &affinity) {
int depth = __kmp_topology->get_depth();
// Create the table of masks, indexed by thread Id.
- unsigned numUnique;
+ unsigned numUnique = 0;
int numAddrs = __kmp_topology->get_num_hw_threads();
// If OMP_PLACES=cores:<attribute> specified, then attempt
// to make OS Id mask table using those attributes
diff --git a/openmp/runtime/src/kmp_affinity.h b/openmp/runtime/src/kmp_affinity.h
index 3dc2c84d53f77..03026f33bc2da 100644
--- a/openmp/runtime/src/kmp_affinity.h
+++ b/openmp/runtime/src/kmp_affinity.h
@@ -29,6 +29,8 @@ class KMPHwlocAffinity : public KMPAffinity {
mask = hwloc_bitmap_alloc();
this->zero();
}
+ Mask(const Mask &other) = delete;
+ Mask &operator=(const Mask &other) = delete;
~Mask() { hwloc_bitmap_free(mask); }
void set(int i) override { hwloc_bitmap_set(mask, i); }
bool is_set(int i) const override { return hwloc_bitmap_isset(mask, i); }
@@ -1269,7 +1271,7 @@ class hierarchy_info {
leaf. It corresponds to the number of entries in numPerLevel if we exclude
all but one trailing 1. */
kmp_uint32 depth;
- kmp_uint32 base_num_threads;
+ kmp_uint32 base_num_threads = 0;
enum init_status { initialized = 0, not_initialized = 1, initializing = 2 };
volatile kmp_int8 uninitialized; // 0=initialized, 1=not initialized,
// 2=initialization in progress
@@ -1279,8 +1281,8 @@ class hierarchy_info {
the parent of a node at level i has. For example, if we have a machine
with 4 packages, 4 cores/package and 2 HT per core, then numPerLevel =
{2, 4, 4, 1, 1}. All empty levels are set to 1. */
- kmp_uint32 *numPerLevel;
- kmp_uint32 *skipPerLevel;
+ kmp_uint32 *numPerLevel = nullptr;
+ kmp_uint32 *skipPerLevel = nullptr;
void deriveLevels() {
int hier_depth = __kmp_topology->get_depth();
diff --git a/openmp/runtime/src/kmp_barrier.cpp b/openmp/runtime/src/kmp_barrier.cpp
index b381694c0953e..661e6218a9136 100644
--- a/openmp/runtime/src/kmp_barrier.cpp
+++ b/openmp/runtime/src/kmp_barrier.cpp
@@ -444,7 +444,8 @@ static void __kmp_dist_barrier_release(
next_go = my_current_iter + distributedBarrier::MAX_ITERS;
my_go_index = tid / b->threads_per_go;
if (this_thr->th.th_used_in_team.load() == 3) {
- KMP_COMPARE_AND_STORE_ACQ32(&(this_thr->th.th_used_in_team), 3, 1);
+ (void)KMP_COMPARE_AND_STORE_ACQ32(&(this_thr->th.th_used_in_team), 3,
+ 1);
}
// Check if go flag is set
if (b->go[my_go_index].go.load() != next_go) {
diff --git a/openmp/runtime/src/kmp_csupport.cpp b/openmp/runtime/src/kmp_csupport.cpp
index f45fe646d1d9a..a08e1ad576015 100644
--- a/openmp/runtime/src/kmp_csupport.cpp
+++ b/openmp/runtime/src/kmp_csupport.cpp
@@ -1545,7 +1545,7 @@ void __kmpc_critical_with_hint(ident_t *loc, kmp_int32 global_tid,
kmp_dyna_lockseq_t lockseq = __kmp_map_hint_to_lock(hint);
if (*lk == 0) {
if (KMP_IS_D_LOCK(lockseq)) {
- KMP_COMPARE_AND_STORE_ACQ32(
+ (void)KMP_COMPARE_AND_STORE_ACQ32(
(volatile kmp_int32 *)&((kmp_base_tas_lock_t *)crit)->poll, 0,
KMP_GET_D_TAG(lockseq));
} else {
@@ -3442,8 +3442,8 @@ __kmp_enter_critical_section_reduce_block(ident_t *loc, kmp_int32 global_tid,
// Check if it is initialized.
if (*lk == 0) {
if (KMP_IS_D_LOCK(__kmp_user_lock_seq)) {
- KMP_COMPARE_AND_STORE_ACQ32((volatile kmp_int32 *)crit, 0,
- KMP_GET_D_TAG(__kmp_user_lock_seq));
+ (void)KMP_COMPARE_AND_STORE_ACQ32((volatile kmp_int32 *)crit, 0,
+ KMP_GET_D_TAG(__kmp_user_lock_seq));
} else {
__kmp_init_indirect_csptr(crit, loc, global_tid,
KMP_GET_I_TAG(__kmp_user_lock_seq));
diff --git a/openmp/runtime/src/kmp_runtime.cpp b/openmp/runtime/src/kmp_runtime.cpp
index 74b44b5d4d9cc..cf73aa066d7fa 100644
--- a/openmp/runtime/src/kmp_runtime.cpp
+++ b/openmp/runtime/src/kmp_runtime.cpp
@@ -1953,8 +1953,8 @@ int __kmp_fork_call(ident_t *loc, int gtid,
#if OMPT_SUPPORT
ompt_data_t ompt_parallel_data = ompt_data_none;
- ompt_data_t *parent_task_data;
- ompt_frame_t *ompt_frame;
+ ompt_data_t *parent_task_data = NULL;
+ ompt_frame_t *ompt_frame = NULL;
void *return_address = NULL;
if (ompt_enabled.enabled) {
@@ -5686,8 +5686,8 @@ void __kmp_free_team(kmp_root_t *root,
for (f = 1; f < team->t.t_nproc; ++f) {
KMP_DEBUG_ASSERT(team->t.t_threads[f]);
if (__kmp_barrier_gather_pattern[bs_forkjoin_barrier] == bp_dist_bar) {
- KMP_COMPARE_AND_STORE_ACQ32(&(team->t.t_threads[f]->th.th_used_in_team),
- 1, 2);
+ (void)KMP_COMPARE_AND_STORE_ACQ32(
+ &(team->t.t_threads[f]->th.th_used_in_team), 1, 2);
}
__kmp_free_thread(team->t.t_threads[f]);
}
@@ -9105,8 +9105,8 @@ void __kmp_add_threads_to_team(kmp_team_t *team, int new_nthreads) {
// to wake it up.
for (int f = 1; f < new_nthreads; ++f) {
KMP_DEBUG_ASSERT(team->t.t_threads[f]);
- KMP_COMPARE_AND_STORE_ACQ32(&(team->t.t_threads[f]->th.th_used_in_team), 0,
- 3);
+ (void)KMP_COMPARE_AND_STORE_ACQ32(
+ &(team->t.t_threads[f]->th.th_used_in_team), 0, 3);
if (__kmp_dflt_blocktime != KMP_MAX_BLOCKTIME) { // Wake up sleeping threads
__kmp_resume_32(team->t.t_threads[f]->th.th_info.ds.ds_gtid,
(kmp_flag_32<false, false> *)NULL);
diff --git a/openmp/runtime/src/kmp_tasking.cpp b/openmp/runtime/src/kmp_tasking.cpp
index 936afb91ac2f8..3bf69901eb136 100644
--- a/openmp/runtime/src/kmp_tasking.cpp
+++ b/openmp/runtime/src/kmp_tasking.cpp
@@ -5276,7 +5276,7 @@ static void __kmp_taskloop(ident_t *loc, int gtid, kmp_task_t *task, int if_val,
switch (sched) {
case 0: // no schedule clause specified, we can choose the default
// let's try to schedule (team_size*10) tasks
- grainsize = thread->th.th_team_nproc * 10;
+ grainsize = thread->th.th_team_nproc * static_cast<kmp_uint64>(10);
KMP_FALLTHROUGH();
case 2: // num_tasks provided
if (grainsize > tc) {
diff --git a/openmp/runtime/src/kmp_wait_release.h b/openmp/runtime/src/kmp_wait_release.h
index 12d5d0677a90a..e273cf26d1e26 100644
--- a/openmp/runtime/src/kmp_wait_release.h
+++ b/openmp/runtime/src/kmp_wait_release.h
@@ -104,7 +104,8 @@ template <> struct flag_traits<flag_oncore> {
template <flag_type FlagType> class kmp_flag {
protected:
flag_properties t; /**< "Type" of the flag in loc */
- kmp_info_t *waiting_threads[1]; /**< Threads sleeping on this thread. */
+ /**< Threads sleeping on this thread. */
+ kmp_info_t *waiting_threads[1] = {nullptr};
kmp_uint32 num_waiting_threads; /**< Num threads sleeping on this thread. */
std::atomic<bool> *sleepLoc;
@@ -140,7 +141,7 @@ template <typename PtrType, flag_type FlagType, bool Sleepable>
class kmp_flag_native : public kmp_flag<FlagType> {
protected:
volatile PtrType *loc;
- PtrType checker; /**< When flag==checker, it has been released. */
+ PtrType checker = (PtrType)0; /**< When flag==checker, it has been released */
typedef flag_traits<FlagType> traits_type;
public:
@@ -234,7 +235,7 @@ template <typename PtrType, flag_type FlagType, bool Sleepable>
class kmp_flag_atomic : public kmp_flag<FlagType> {
protected:
std::atomic<PtrType> *loc; /**< Pointer to flag location to wait on */
- PtrType checker; /**< Flag == checker means it has been released. */
+ PtrType checker = (PtrType)0; /**< Flag==checker means it has been released */
public:
typedef flag_traits<FlagType> traits_type;
typedef PtrType flag_t;
@@ -931,7 +932,8 @@ class kmp_flag_oncore : public kmp_flag_native<kmp_uint64, flag_oncore, false> {
kmp_uint32 offset; /**< Portion of flag of interest for an operation. */
bool flag_switch; /**< Indicates a switch in flag location. */
enum barrier_type bt; /**< Barrier type. */
- kmp_info_t *this_thr; /**< Thread to redirect to different flag location. */
+ /**< Thread to redirect to different flag location. */
+ kmp_info_t *this_thr = nullptr;
#if USE_ITT_BUILD
void *itt_sync_obj; /**< ITT object to pass to new flag location. */
#endif
diff --git a/openmp/runtime/src/ompt-general.cpp b/openmp/runtime/src/ompt-general.cpp
index 95aab6cd79e5a..453e247ce5634 100644
--- a/openmp/runtime/src/ompt-general.cpp
+++ b/openmp/runtime/src/ompt-general.cpp
@@ -104,9 +104,11 @@ static ompt_start_tool_result_t *ompt_start_tool_result = NULL;
#if KMP_OS_WINDOWS
static HMODULE ompt_tool_module = NULL;
+static HMODULE ompt_archer_module = NULL;
#define OMPT_DLCLOSE(Lib) FreeLibrary(Lib)
#else
static void *ompt_tool_module = NULL;
+static void *ompt_archer_module = NULL;
#define OMPT_DLCLOSE(Lib) dlclose(Lib)
#endif
@@ -374,6 +376,7 @@ ompt_try_start_tool(unsigned int omp_version, const char *runtime_version) {
"Tool was started and is using the OMPT interface.\n");
OMPT_VERBOSE_INIT_PRINT(
"----- END LOGGING OF TOOL REGISTRATION -----\n");
+ ompt_archer_module = h;
return ret;
}
OMPT_VERBOSE_INIT_CONTINUED_PRINT(
@@ -381,6 +384,7 @@ ompt_try_start_tool(unsigned int omp_version, const char *runtime_version) {
} else {
OMPT_VERBOSE_INIT_CONTINUED_PRINT("Failed: %s\n", dlerror());
}
+ OMPT_DLCLOSE(h);
}
}
#endif
@@ -521,6 +525,8 @@ void ompt_fini() {
}
}
+ if (ompt_archer_module)
+ OMPT_DLCLOSE(ompt_archer_module);
if (ompt_tool_module)
OMPT_DLCLOSE(ompt_tool_module);
memset(&ompt_enabled, 0, sizeof(ompt_enabled));
More information about the Openmp-commits
mailing list