From 398c5a6fdc4f2df9cd291db8908d47050d69eb23 Mon Sep 17 00:00:00 2001 From: Christian Trott Date: Mon, 31 Jul 2023 16:19:50 -0600 Subject: [PATCH] Fix an issue in simple-kernel-timer with mismatched linked symbols Basic Kokkos Serial build and test failed for me on MacOS. Symptom was that the simple kernel timer "currentEntry" symbol was null during end of parallel for call back. Tracked that down to that symbol having different addresses inside the increment_counter function and the end of parallel_for callback. I believe moving the increment_counter functions to not be inlined may help? Or do we need accessor functions which are uniform? At least this change did fix the issue for me. --- profiling/simple-kernel-timer/kp_shared.cpp | 33 +++++++++++++++++++ profiling/simple-kernel-timer/kp_shared.h | 35 ++------------------- 2 files changed, 35 insertions(+), 33 deletions(-) diff --git a/profiling/simple-kernel-timer/kp_shared.cpp b/profiling/simple-kernel-timer/kp_shared.cpp index 43f306d81..7b7928046 100644 --- a/profiling/simple-kernel-timer/kp_shared.cpp +++ b/profiling/simple-kernel-timer/kp_shared.cpp @@ -27,5 +27,38 @@ char* outputDelimiter; int current_region_level = 0; KernelPerformanceInfo* regions[512]; +void increment_counter(const char* name, KernelExecutionType kType) { + std::string nameStr(name); + + if (count_map.find(name) == count_map.end()) { + KernelPerformanceInfo* info = new KernelPerformanceInfo(nameStr, kType); + count_map.insert( + std::pair(nameStr, info)); + + currentEntry = info; + } else { + currentEntry = count_map[nameStr]; + } + + currentEntry->startTimer(); +} + +void increment_counter_region(const char* name, KernelExecutionType kType) { + std::string nameStr(name); + + if (count_map.find(name) == count_map.end()) { + KernelPerformanceInfo* info = new KernelPerformanceInfo(nameStr, kType); + count_map.insert( + std::pair(nameStr, info)); + + regions[current_region_level] = info; + } else { + regions[current_region_level] = count_map[nameStr]; + } + + regions[current_region_level]->startTimer(); + current_region_level++; +} + } // namespace KernelTimer } // namespace KokkosTools diff --git a/profiling/simple-kernel-timer/kp_shared.h b/profiling/simple-kernel-timer/kp_shared.h index 990fee2a7..f719f5b20 100644 --- a/profiling/simple-kernel-timer/kp_shared.h +++ b/profiling/simple-kernel-timer/kp_shared.h @@ -31,39 +31,8 @@ extern char* outputDelimiter; extern int current_region_level; extern KernelPerformanceInfo* regions[512]; -inline void increment_counter(const char* name, KernelExecutionType kType) { - std::string nameStr(name); - - if (count_map.find(name) == count_map.end()) { - KernelPerformanceInfo* info = new KernelPerformanceInfo(nameStr, kType); - count_map.insert( - std::pair(nameStr, info)); - - currentEntry = info; - } else { - currentEntry = count_map[nameStr]; - } - - currentEntry->startTimer(); -} - -inline void increment_counter_region(const char* name, - KernelExecutionType kType) { - std::string nameStr(name); - - if (count_map.find(name) == count_map.end()) { - KernelPerformanceInfo* info = new KernelPerformanceInfo(nameStr, kType); - count_map.insert( - std::pair(nameStr, info)); - - regions[current_region_level] = info; - } else { - regions[current_region_level] = count_map[nameStr]; - } - - regions[current_region_level]->startTimer(); - current_region_level++; -} +void increment_counter(const char* name, KernelExecutionType kType); +void increment_counter_region(const char* name, KernelExecutionType kType); inline bool compareKernelPerformanceInfo(KernelPerformanceInfo* left, KernelPerformanceInfo* right) {