Skip to content

[OpenMP][OMPT][OMPD] Fix frame flags for OpenMP tool APIs #114118

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 27, 2025

Conversation

jprotze
Copy link
Collaborator

@jprotze jprotze commented Oct 29, 2024

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

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 llvm#89058
@llvmbot llvmbot added the openmp:libomp OpenMP host runtime label Oct 29, 2024
@jprotze jprotze requested a review from hansangbae October 29, 2024 19:22
Copy link
Contributor

@hansangbae hansangbae left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jprotze
Copy link
Collaborator Author

jprotze commented Oct 31, 2024

Thanks for the review. I'll give @jdelsign some time to check that this patch indeed fixes his issues reported in #89058 .

@jprotze jprotze merged commit 12a9e2a into llvm:main Feb 27, 2025
8 checks passed
joaosaffran pushed a commit to joaosaffran/llvm-project that referenced this pull request Mar 3, 2025
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 llvm#89058
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
openmp:libomp OpenMP host runtime
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[OpenMP][OMPD] LLVM OMPD library ompd_get_task_frame() returns garbage in the frame_flag field
3 participants