This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP][OMPT] Update the omp-tools header file to reflect 5.1 changes
ClosedPublic

Authored by protze.joachim on Nov 4 2020, 2:35 AM.

Details

Summary

This doesn't add functionality, but just adds the new types and renames the master callback to masked callback.

Diff Detail

Event Timeline

protze.joachim created this revision.Nov 4 2020, 2:35 AM
protze.joachim requested review of this revision.Nov 4 2020, 2:35 AM

This implies also some changes to the OMPT tests

jdoerfert accepted this revision.Nov 9 2020, 8:19 AM

LGTM, one nit below.

openmp/runtime/test/ompt/synchronization/taskgroup.c
32 ↗(On Diff #302798)

No replacement?

This revision is now accepted and ready to land.Nov 9 2020, 8:19 AM
protze.joachim marked an inline comment as done.Nov 10 2020, 4:50 AM
protze.joachim added inline comments.
openmp/runtime/test/ompt/synchronization/taskgroup.c
32 ↗(On Diff #302798)

The test does not use the callback, so I removed the check rather than replacing it.

protze.joachim updated this revision to Diff 304521.EditedNov 11 2020, 8:12 AM
protze.joachim marked an inline comment as done.

The ompt_callback_master_t type is deprecated, but not removed. So, omp-tools.h still needs to define the type.
I moved the existing master.c test to masked.c and added a new master.c test, which just tries to register the deprecated callback.

I also ran all tests with cmake -DOPENMP_TEST_FLAGS:STRING="-Werror -Wno-#warnings" and fixed all warnings for the ompt test cases (mainly about missing cases in switch statements).

lxfind added a subscriber: lxfind.Nov 11 2020, 4:40 PM

Are all uses of on_ompt_callback_master expected to be replaced?
I see this is still using it:
https://github.com/llvm/llvm-project/blob/master/openmp/tools/multiplex/tests/custom_data_storage/first-tool.h#L126

Are all uses of on_ompt_callback_master expected to be replaced?
I see this is still using it:
https://github.com/llvm/llvm-project/blob/master/openmp/tools/multiplex/tests/custom_data_storage/first-tool.h#L126

multiplex tests are fixed in 25b3164bfbe6dc