[Openmp-commits] [openmp] [OpenMP][OMPT][OMPD] Fix frame flags for OpenMP tool APIs (PR #114118)
via Openmp-commits
openmp-commits at lists.llvm.org
Tue Oct 29 12:21:59 PDT 2024
https://github.com/jprotze created https://github.com/llvm/llvm-project/pull/114118
In several cases the flags entries in ompt_frame_t are not initialized. According to @jdelsign the address provided as reenter and exit address is the canonical frame address (cfa) rather than a "framepointer". This patch makes sure that the flags entry is always initialized and changes the value from ompt_frame_framepointer to ompt_frame_cfa.
The assertion in the tests makes sure that the flags are always set, when a tool (callback.h in this case) looks at the value.
Fixes #89058
>From 22d02b63ac3bc6113e532ec6069afa3dd70a417d Mon Sep 17 00:00:00 2001
From: Joachim Jenke <jenke at itc.rwth-aachen.de>
Date: Tue, 29 Oct 2024 20:13:00 +0100
Subject: [PATCH] [OpenMP][OMPT][OMPD] Fix frame flags for OpenMP tool APIs
In several cases the flags entries in ompt_frame_t are not initialized.
According to @jdelsign the address provided as reenter and exit address
is the canonical frame address (cfa) rather than a "framepointer".
This patch makes sure that the flags is always initialized and changes
the value from ompt_frame_framepointer to ompt_frame_cfa.
The assertion in the tests makes sure that the flags are always set, when
a tool (callback.h in this case) looks at the value.
Fixes #89058
---
openmp/runtime/src/kmp_tasking.cpp | 22 +++-------------------
openmp/runtime/src/ompt-general.cpp | 1 +
openmp/runtime/src/ompt-internal.h | 2 ++
openmp/runtime/src/ompt-specific.cpp | 2 ++
openmp/runtime/src/ompt-specific.h | 15 +++++++++++++++
openmp/runtime/test/ompt/callback.h | 22 +++++++++++++++++++++-
6 files changed, 44 insertions(+), 20 deletions(-)
diff --git a/openmp/runtime/src/kmp_tasking.cpp b/openmp/runtime/src/kmp_tasking.cpp
index 932799e133b45b..3e229b517cfcd6 100644
--- a/openmp/runtime/src/kmp_tasking.cpp
+++ b/openmp/runtime/src/kmp_tasking.cpp
@@ -714,22 +714,6 @@ static void __kmp_task_start(kmp_int32 gtid, kmp_task_t *task,
#if OMPT_SUPPORT
//------------------------------------------------------------------------------
-// __ompt_task_init:
-// Initialize OMPT fields maintained by a task. This will only be called after
-// ompt_start_tool, so we already know whether ompt is enabled or not.
-
-static inline void __ompt_task_init(kmp_taskdata_t *task, int tid) {
- // The calls to __ompt_task_init already have the ompt_enabled condition.
- task->ompt_task_info.task_data.value = 0;
- task->ompt_task_info.frame.exit_frame = ompt_data_none;
- task->ompt_task_info.frame.enter_frame = ompt_data_none;
- task->ompt_task_info.frame.exit_frame_flags =
- ompt_frame_runtime | ompt_frame_framepointer;
- task->ompt_task_info.frame.enter_frame_flags =
- ompt_frame_runtime | ompt_frame_framepointer;
- task->ompt_task_info.dispatch_chunk.start = 0;
- task->ompt_task_info.dispatch_chunk.iterations = 0;
-}
// __ompt_task_start:
// Build and trigger task-begin event
@@ -804,7 +788,7 @@ static void __kmpc_omp_task_begin_if0_template(ident_t *loc_ref, kmp_int32 gtid,
taskdata->ompt_task_info.frame.exit_frame.ptr = frame_address;
current_task->ompt_task_info.frame.enter_frame_flags =
taskdata->ompt_task_info.frame.exit_frame_flags =
- ompt_frame_application | ompt_frame_framepointer;
+ OMPT_FRAME_FLAGS_APP;
}
if (ompt_enabled.ompt_callback_task_create) {
ompt_task_info_t *parent_info = &(current_task->ompt_task_info);
@@ -1268,8 +1252,7 @@ static void __kmpc_omp_task_complete_if0_template(ident_t *loc_ref,
ompt_frame_t *ompt_frame;
__ompt_get_task_info_internal(0, NULL, NULL, &ompt_frame, NULL, NULL);
ompt_frame->enter_frame = ompt_data_none;
- ompt_frame->enter_frame_flags =
- ompt_frame_runtime | ompt_frame_framepointer;
+ ompt_frame->enter_frame_flags = OMPT_FRAME_FLAGS_RUNTIME;
}
#endif
@@ -2010,6 +1993,7 @@ kmp_int32 __kmpc_omp_task_parts(ident_t *loc_ref, kmp_int32 gtid,
#if OMPT_SUPPORT
if (UNLIKELY(ompt_enabled.enabled)) {
parent->ompt_task_info.frame.enter_frame = ompt_data_none;
+ parent->ompt_task_info.frame.enter_frame_flags = OMPT_FRAME_FLAGS_RUNTIME;
}
#endif
return TASK_CURRENT_NOT_QUEUED;
diff --git a/openmp/runtime/src/ompt-general.cpp b/openmp/runtime/src/ompt-general.cpp
index 923eea2a563a91..64f83a5cb19cec 100644
--- a/openmp/runtime/src/ompt-general.cpp
+++ b/openmp/runtime/src/ompt-general.cpp
@@ -497,6 +497,7 @@ void ompt_post_init() {
kmp_info_t *root_thread = ompt_get_thread();
ompt_set_thread_state(root_thread, ompt_state_overhead);
+ __ompt_task_init(root_thread->th.th_current_task, 0);
if (ompt_enabled.ompt_callback_thread_begin) {
ompt_callbacks.ompt_callback(ompt_callback_thread_begin)(
diff --git a/openmp/runtime/src/ompt-internal.h b/openmp/runtime/src/ompt-internal.h
index 580a7c2ac79168..0cfab8bfaa1906 100644
--- a/openmp/runtime/src/ompt-internal.h
+++ b/openmp/runtime/src/ompt-internal.h
@@ -111,6 +111,8 @@ void ompt_fini(void);
#define OMPT_GET_RETURN_ADDRESS(level) __builtin_return_address(level)
#define OMPT_GET_FRAME_ADDRESS(level) __builtin_frame_address(level)
+#define OMPT_FRAME_FLAGS_APP (ompt_frame_application | ompt_frame_cfa)
+#define OMPT_FRAME_FLAGS_RUNTIME (ompt_frame_runtime | ompt_frame_cfa)
int __kmp_control_tool(uint64_t command, uint64_t modifier, void *arg);
diff --git a/openmp/runtime/src/ompt-specific.cpp b/openmp/runtime/src/ompt-specific.cpp
index 0737c0cdfb1602..94ae2e52938751 100644
--- a/openmp/runtime/src/ompt-specific.cpp
+++ b/openmp/runtime/src/ompt-specific.cpp
@@ -266,6 +266,8 @@ void __ompt_lw_taskteam_init(ompt_lw_taskteam_t *lwt, kmp_info_t *thr, int gtid,
lwt->ompt_task_info.task_data.value = 0;
lwt->ompt_task_info.frame.enter_frame = ompt_data_none;
lwt->ompt_task_info.frame.exit_frame = ompt_data_none;
+ lwt->ompt_task_info.frame.enter_frame_flags = OMPT_FRAME_FLAGS_RUNTIME;
+ lwt->ompt_task_info.frame.exit_frame_flags = OMPT_FRAME_FLAGS_RUNTIME;
lwt->ompt_task_info.scheduling_parent = NULL;
lwt->heap = 0;
lwt->parent = 0;
diff --git a/openmp/runtime/src/ompt-specific.h b/openmp/runtime/src/ompt-specific.h
index 7864ed6126c701..e9e40d43429eaf 100644
--- a/openmp/runtime/src/ompt-specific.h
+++ b/openmp/runtime/src/ompt-specific.h
@@ -54,6 +54,21 @@ int __ompt_get_task_info_internal(int ancestor_level, int *type,
ompt_data_t *__ompt_get_thread_data_internal();
+// __ompt_task_init:
+// Initialize OMPT fields maintained by a task. This will only be called after
+// ompt_start_tool, so we already know whether ompt is enabled or not.
+
+static inline void __ompt_task_init(kmp_taskdata_t *task, int tid) {
+ // The calls to __ompt_task_init already have the ompt_enabled condition.
+ task->ompt_task_info.task_data.value = 0;
+ task->ompt_task_info.frame.exit_frame = ompt_data_none;
+ task->ompt_task_info.frame.enter_frame = ompt_data_none;
+ task->ompt_task_info.frame.exit_frame_flags =
+ task->ompt_task_info.frame.enter_frame_flags = OMPT_FRAME_FLAGS_RUNTIME;
+ task->ompt_task_info.dispatch_chunk.start = 0;
+ task->ompt_task_info.dispatch_chunk.iterations = 0;
+}
+
/*
* Unused currently
static uint64_t __ompt_get_get_unique_id_internal();
diff --git a/openmp/runtime/test/ompt/callback.h b/openmp/runtime/test/ompt/callback.h
index 07d38cf836dff0..4dd1db4c4225b3 100644
--- a/openmp/runtime/test/ompt/callback.h
+++ b/openmp/runtime/test/ompt/callback.h
@@ -12,6 +12,8 @@
#include <omp.h>
#include <omp-tools.h>
#include "ompt-signal.h"
+#include <stdlib.h>
+#include <assert.h>
// Used to detect architecture
#include "../../src/kmp_platform.h"
@@ -147,6 +149,22 @@ static ompt_get_proc_id_t ompt_get_proc_id;
static ompt_enumerate_states_t ompt_enumerate_states;
static ompt_enumerate_mutex_impls_t ompt_enumerate_mutex_impls;
+void assert_frame_flags(int enterf, int exitf) {
+ if (!(enterf == (ompt_frame_application | ompt_frame_cfa) ||
+ enterf == (ompt_frame_runtime | ompt_frame_cfa))) {
+ printf("enter_frame_flags (%i) is invalid\n", enterf);
+ fflush(NULL);
+ }
+ if (!(exitf == (ompt_frame_application | ompt_frame_cfa) ||
+ exitf == (ompt_frame_runtime | ompt_frame_cfa))) {
+ printf("exit_frame_flags (%i) is invalid\n", exitf);
+ fflush(NULL);
+ }
+ assert(enterf == (ompt_frame_application | ompt_frame_cfa) ||
+ enterf == (ompt_frame_runtime | ompt_frame_cfa));
+ assert(exitf == (ompt_frame_application | ompt_frame_cfa) ||
+ exitf == (ompt_frame_runtime | ompt_frame_cfa));
+}
static void print_ids(int level)
{
int task_type, thread_num;
@@ -157,7 +175,7 @@ static void print_ids(int level)
&task_parallel_data, &thread_num);
char buffer[2048];
format_task_type(task_type, buffer);
- if (frame)
+ if (frame) {
printf("%" PRIu64 ": task level %d: parallel_id=%" PRIu64
", task_id=%" PRIu64 ", exit_frame=%p, reenter_frame=%p, "
"task_type=%s=%d, thread_num=%d\n",
@@ -165,6 +183,8 @@ static void print_ids(int level)
exists_task ? task_parallel_data->value : 0,
exists_task ? task_data->value : 0, frame->exit_frame.ptr,
frame->enter_frame.ptr, buffer, task_type, thread_num);
+ assert_frame_flags(frame->enter_frame_flags, frame->exit_frame_flags);
+ }
}
#define get_frame_address(level) __builtin_frame_address(level)
More information about the Openmp-commits
mailing list