From 10f3296dd7d74c975f208a8569221dc8f96d1db1 Mon Sep 17 00:00:00 2001 From: Alexandre Ganea <37383324+aganea@users.noreply.github.com> Date: Tue, 23 Jan 2024 08:38:18 -0500 Subject: [PATCH] [openmp] Fix warnings when building on Windows with latest MSVC or Clang ToT (#77853) There were quite a few compilation warnings when building openmp on Windows with the latest Visual Studios 2022 version 17.8.4. Some other warnings were visible with the latest Clang at tip. This commit fixes all of them. --- openmp/cmake/HandleOpenMPOptions.cmake | 9 ++++++++ openmp/runtime/src/kmp_affinity.cpp | 27 ++++++++++++++++++------ openmp/runtime/src/kmp_barrier.cpp | 8 +++---- openmp/runtime/src/kmp_io.cpp | 19 ----------------- openmp/runtime/src/kmp_os.h | 2 ++ openmp/runtime/src/kmp_settings.cpp | 21 +++++++++++++++++- openmp/runtime/src/kmp_wait_release.h | 5 +++-- openmp/runtime/src/z_Windows_NT_util.cpp | 3 ++- 8 files changed, 60 insertions(+), 34 deletions(-) diff --git a/openmp/cmake/HandleOpenMPOptions.cmake b/openmp/cmake/HandleOpenMPOptions.cmake index b0fd0b7de4bf..201aeabbd3df 100644 --- a/openmp/cmake/HandleOpenMPOptions.cmake +++ b/openmp/cmake/HandleOpenMPOptions.cmake @@ -41,3 +41,12 @@ append_if(OPENMP_HAVE_WMAYBE_UNINITIALIZED_FLAG "-Wno-maybe-uninitialized" CMAKE append_if(OPENMP_HAVE_NO_SEMANTIC_INTERPOSITION "-fno-semantic-interposition" CMAKE_C_FLAGS CMAKE_CXX_FLAGS) append_if(OPENMP_HAVE_FUNCTION_SECTIONS "-ffunction-section" CMAKE_C_FLAGS CMAKE_CXX_FLAGS) append_if(OPENMP_HAVE_DATA_SECTIONS "-fdata-sections" CMAKE_C_FLAGS CMAKE_CXX_FLAGS) + +if (MSVC) + # Disable "warning C4201: nonstandard extension used: nameless struct/union" + append("-wd4201" CMAKE_C_FLAGS CMAKE_CXX_FLAGS) + + # Disable "warning C4190: '__kmpc_atomic_cmplx8_rd' has C-linkage specified, but returns + # UDT '__kmp_cmplx64_t' which is incompatible with C" + append("-wd4190" CMAKE_C_FLAGS CMAKE_CXX_FLAGS) +endif() diff --git a/openmp/runtime/src/kmp_affinity.cpp b/openmp/runtime/src/kmp_affinity.cpp index 7009730a49ba..d9e53bf946af 100644 --- a/openmp/runtime/src/kmp_affinity.cpp +++ b/openmp/runtime/src/kmp_affinity.cpp @@ -28,6 +28,8 @@ #endif #include +#include "llvm/Support/Compiler.h" + // The machine topology kmp_topology_t *__kmp_topology = nullptr; // KMP_HW_SUBSET environment variable @@ -127,8 +129,12 @@ const char *__kmp_hw_get_catalog_string(kmp_hw_t type, bool plural) { return ((plural) ? KMP_I18N_STR(Threads) : KMP_I18N_STR(Thread)); case KMP_HW_PROC_GROUP: return ((plural) ? KMP_I18N_STR(ProcGroups) : KMP_I18N_STR(ProcGroup)); + case KMP_HW_UNKNOWN: + case KMP_HW_LAST: + return KMP_I18N_STR(Unknown); } - return KMP_I18N_STR(Unknown); + KMP_ASSERT2(false, "Unhandled kmp_hw_t enumeration"); + LLVM_BUILTIN_UNREACHABLE; } const char *__kmp_hw_get_keyword(kmp_hw_t type, bool plural) { @@ -157,13 +163,18 @@ const char *__kmp_hw_get_keyword(kmp_hw_t type, bool plural) { return ((plural) ? "threads" : "thread"); case KMP_HW_PROC_GROUP: return ((plural) ? "proc_groups" : "proc_group"); + case KMP_HW_UNKNOWN: + case KMP_HW_LAST: + return ((plural) ? "unknowns" : "unknown"); } - return ((plural) ? "unknowns" : "unknown"); + KMP_ASSERT2(false, "Unhandled kmp_hw_t enumeration"); + LLVM_BUILTIN_UNREACHABLE; } const char *__kmp_hw_get_core_type_string(kmp_hw_core_type_t type) { switch (type) { case KMP_HW_CORE_TYPE_UNKNOWN: + case KMP_HW_MAX_NUM_CORE_TYPES: return "unknown"; #if KMP_ARCH_X86 || KMP_ARCH_X86_64 case KMP_HW_CORE_TYPE_ATOM: @@ -172,7 +183,8 @@ const char *__kmp_hw_get_core_type_string(kmp_hw_core_type_t type) { return "Intel(R) Core(TM) processor"; #endif } - return "unknown"; + KMP_ASSERT2(false, "Unhandled kmp_hw_core_type_t enumeration"); + LLVM_BUILTIN_UNREACHABLE; } #if KMP_AFFINITY_SUPPORTED @@ -1238,17 +1250,18 @@ bool kmp_topology_t::filter_hw_subset() { struct core_type_indexer { int operator()(const kmp_hw_thread_t &t) const { switch (t.attrs.get_core_type()) { + case KMP_HW_CORE_TYPE_UNKNOWN: + case KMP_HW_MAX_NUM_CORE_TYPES: + return 0; #if KMP_ARCH_X86 || KMP_ARCH_X86_64 case KMP_HW_CORE_TYPE_ATOM: return 1; case KMP_HW_CORE_TYPE_CORE: return 2; #endif - case KMP_HW_CORE_TYPE_UNKNOWN: - return 0; } - KMP_ASSERT(0); - return 0; + KMP_ASSERT2(false, "Unhandled kmp_hw_thread_t enumeration"); + LLVM_BUILTIN_UNREACHABLE; } }; struct core_eff_indexer { diff --git a/openmp/runtime/src/kmp_barrier.cpp b/openmp/runtime/src/kmp_barrier.cpp index 281b8e9c2883..e9ab15f1723b 100644 --- a/openmp/runtime/src/kmp_barrier.cpp +++ b/openmp/runtime/src/kmp_barrier.cpp @@ -2403,11 +2403,11 @@ void __kmp_fork_barrier(int gtid, int tid) { #if USE_ITT_BUILD void *itt_sync_obj = NULL; #endif /* USE_ITT_BUILD */ +#ifdef KMP_DEBUG if (team) - - KA_TRACE(10, ("__kmp_fork_barrier: T#%d(%d:%d) has arrived\n", gtid, - (team != NULL) ? team->t.t_id : -1, tid)); - + KA_TRACE(10, ("__kmp_fork_barrier: T#%d(%d:%d) has arrived\n", gtid, + (team != NULL) ? team->t.t_id : -1, tid)); +#endif // th_team pointer only valid for primary thread here if (KMP_MASTER_TID(tid)) { #if USE_ITT_BUILD && USE_ITT_NOTIFY diff --git a/openmp/runtime/src/kmp_io.cpp b/openmp/runtime/src/kmp_io.cpp index 578e6e671cdf..0c52662bc235 100644 --- a/openmp/runtime/src/kmp_io.cpp +++ b/openmp/runtime/src/kmp_io.cpp @@ -50,24 +50,6 @@ static HANDLE __kmp_stderr = NULL; static int __kmp_console_exists = FALSE; static kmp_str_buf_t __kmp_console_buf; -static int is_console(void) { - char buffer[128]; - DWORD rc = 0; - DWORD err = 0; - // Try to get console title. - SetLastError(0); - // GetConsoleTitle does not reset last error in case of success or short - // buffer, so we need to clear it explicitly. - rc = GetConsoleTitle(buffer, sizeof(buffer)); - if (rc == 0) { - // rc == 0 means getting console title failed. Let us find out why. - err = GetLastError(); - // err == 0 means buffer too short (we suppose console exists). - // In Window applications we usually have err == 6 (invalid handle). - } - return rc > 0 || err == 0; -} - void __kmp_close_console(void) { /* wait until user presses return before closing window */ /* TODO only close if a window was opened */ @@ -84,7 +66,6 @@ void __kmp_close_console(void) { static void __kmp_redirect_output(void) { __kmp_acquire_bootstrap_lock(&__kmp_console_lock); - (void)is_console; if (!__kmp_console_exists) { HANDLE ho; HANDLE he; diff --git a/openmp/runtime/src/kmp_os.h b/openmp/runtime/src/kmp_os.h index 6862fd89b630..025304caf872 100644 --- a/openmp/runtime/src/kmp_os.h +++ b/openmp/runtime/src/kmp_os.h @@ -306,6 +306,8 @@ template <> struct traits_t { !KMP_MIC) #if KMP_OS_WINDOWS +// Don't include everything related to NT status code, we'll do that explicitly +#define WIN32_NO_STATUS #include static inline int KMP_GET_PAGE_SIZE(void) { diff --git a/openmp/runtime/src/kmp_settings.cpp b/openmp/runtime/src/kmp_settings.cpp index 30a4c05fe76b..a9bbfdaf841b 100644 --- a/openmp/runtime/src/kmp_settings.cpp +++ b/openmp/runtime/src/kmp_settings.cpp @@ -29,6 +29,8 @@ #include "ompd-specific.h" #endif +#include "llvm/Support/Compiler.h" + static int __kmp_env_toPrint(char const *name, int flag); bool __kmp_env_format = 0; // 0 - old format; 1 - new format @@ -873,6 +875,10 @@ static void __kmp_stg_print_wait_policy(kmp_str_buf_t *buffer, char const *name, case library_throughput: { value = "PASSIVE"; } break; + case library_none: + case library_serial: { + value = NULL; + } break; } } else { switch (__kmp_library) { @@ -885,6 +891,9 @@ static void __kmp_stg_print_wait_policy(kmp_str_buf_t *buffer, char const *name, case library_throughput: { value = "throughput"; } break; + case library_none: { + value = NULL; + } break; } } if (value != NULL) { @@ -2004,6 +2013,7 @@ static inline const char * __kmp_hw_get_core_type_keyword(kmp_hw_core_type_t type) { switch (type) { case KMP_HW_CORE_TYPE_UNKNOWN: + case KMP_HW_MAX_NUM_CORE_TYPES: return "unknown"; #if KMP_ARCH_X86 || KMP_ARCH_X86_64 case KMP_HW_CORE_TYPE_ATOM: @@ -2012,7 +2022,8 @@ __kmp_hw_get_core_type_keyword(kmp_hw_core_type_t type) { return "intel_core"; #endif } - return "unknown"; + KMP_ASSERT2(false, "Unhandled kmp_hw_core_type_t enumeration"); + LLVM_BUILTIN_UNREACHABLE; } #if KMP_AFFINITY_SUPPORTED @@ -4428,6 +4439,10 @@ static void __kmp_stg_print_omp_schedule(kmp_str_buf_t *buffer, case kmp_sch_auto: __kmp_str_buf_print(buffer, "%s,%d'\n", "auto", __kmp_chunk); break; + default: + KMP_ASSERT2(false, "Unhandled sched_type enumeration"); + LLVM_BUILTIN_UNREACHABLE; + break; } } else { switch (sched) { @@ -4453,6 +4468,10 @@ static void __kmp_stg_print_omp_schedule(kmp_str_buf_t *buffer, case kmp_sch_auto: __kmp_str_buf_print(buffer, "%s'\n", "auto"); break; + default: + KMP_ASSERT2(false, "Unhandled sched_type enumeration"); + LLVM_BUILTIN_UNREACHABLE; + break; } } } // __kmp_stg_print_omp_schedule diff --git a/openmp/runtime/src/kmp_wait_release.h b/openmp/runtime/src/kmp_wait_release.h index 3fcae5687d12..c1a4c778b4b4 100644 --- a/openmp/runtime/src/kmp_wait_release.h +++ b/openmp/runtime/src/kmp_wait_release.h @@ -20,6 +20,8 @@ #include "ompt-specific.h" #endif +#include "llvm/Support/Compiler.h" + /*! @defgroup WAIT_RELEASE Wait/Release operations @@ -1038,7 +1040,6 @@ static inline void __kmp_null_resume_wrapper(kmp_info_t *thr) { case flag_oncore: __kmp_resume_oncore(gtid, RCAST(kmp_flag_oncore *, flag)); break; -#ifdef KMP_DEBUG case flag_unset: KF_TRACE(100, ("__kmp_null_resume_wrapper: flag type %d is unset\n", type)); break; @@ -1046,7 +1047,7 @@ static inline void __kmp_null_resume_wrapper(kmp_info_t *thr) { KF_TRACE(100, ("__kmp_null_resume_wrapper: flag type %d does not match any " "known flag type\n", type)); -#endif + LLVM_BUILTIN_UNREACHABLE; } } diff --git a/openmp/runtime/src/z_Windows_NT_util.cpp b/openmp/runtime/src/z_Windows_NT_util.cpp index 9e264ab45b87..d75b48b2c1bc 100644 --- a/openmp/runtime/src/z_Windows_NT_util.cpp +++ b/openmp/runtime/src/z_Windows_NT_util.cpp @@ -22,6 +22,7 @@ number of running threads in the system. */ #include // UNICODE_STRING +#undef WIN32_NO_STATUS #include #include #ifdef _MSC_VER @@ -1635,7 +1636,7 @@ int __kmp_get_load_balance(int max) { // threads on all cores. So, we don't consider the running threads of this // process. if (pid != 0) { - for (int i = 0; i < num; ++i) { + for (ULONG i = 0; i < num; ++i) { THREAD_STATE state = spi->Threads[i].State; // Count threads that have Ready or Running state. // !!! TODO: Why comment does not match the code???