-
Notifications
You must be signed in to change notification settings - Fork 14.5k
[OpenMP] Add ompTest library to OpenMP #147381
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
base: main
Are you sure you want to change the base?
Conversation
While being aware that this change is of substantial size, we chose to start the reviewing process to gather feedback. Also, please pull in other reviewers as needed. |
It seems we forgot to add the appropriate license statement in the source files. |
Description =========== OpenMP Tooling Interface Testing Library (ompTest) ompTest is a unit testing framework for testing OpenMP implementations. It offers a simple-to-use framework that allows a tester to check for OMPT events in addition to regular unit testing code, supported by linking against GoogleTest by default. It also facilitates writing concise tests while bridging the semantic gap between the unit under test and the OMPT-event testing. Background ========== This library has been developed to provide the means of testing OMPT implementations with reasonable effort. Especially, asynchronous or unordered events are supported and can be verified with ease, which may prove to be challenging with LIT-based tests. Additionally, since the assertions are part of the code being tested, ompTest can reference all corresponding variables during assertion. Basic Usage =========== OMPT event assertions are placed before the code, which shall be tested. These assertion can either be provided as one block or interleaved with the test code. There are two types of asserters: (1) sequenced "order-sensitive" and (2) set "unordered" assserters. Once the test is being run, the corresponding events are triggered by the OpenMP runtime and can be observed. Each of these observed events notifies asserters, which then determine if the test should pass or fail. Example (partial, interleaved) ============================== int N = 100000; int a[N]; int b[N]; OMPT_ASSERT_SEQUENCE(Target, TARGET, BEGIN, 0); OMPT_ASSERT_SEQUENCE(TargetDataOp, ALLOC, N * sizeof(int)); // a ? OMPT_ASSERT_SEQUENCE(TargetDataOp, H2D, N * sizeof(int), &a); OMPT_ASSERT_SEQUENCE(TargetDataOp, ALLOC, N * sizeof(int)); // b ? OMPT_ASSERT_SEQUENCE(TargetDataOp, H2D, N * sizeof(int), &b); OMPT_ASSERT_SEQUENCE(TargetSubmit, 1); OMPT_ASSERT_SEQUENCE(TargetDataOp, D2H, N * sizeof(int), nullptr, &b); OMPT_ASSERT_SEQUENCE(TargetDataOp, D2H, N * sizeof(int), nullptr, &a); OMPT_ASSERT_SEQUENCE(TargetDataOp, DELETE); OMPT_ASSERT_SEQUENCE(TargetDataOp, DELETE); OMPT_ASSERT_SEQUENCE(Target, TARGET, END, 0); { for (int j = 0; j < N; j++) a[j] = b[j]; } References ========== This work has been presented at SC'24 workshops, see: https://ieeexplore.ieee.org/document/10820689 Current State and Future Work ============================= ompTest's development was mostly device-centric and aimed at OMPT device callbacks and device-side tracing. Consequentially, a substantial part of host-related events or features may not be supported in its current state. However, we are confident that the related functionality can be added and ompTest provides a general foundation for future OpenMP and especially OMPT testing. This PR will allow us to upstream the corresponding features, like OMPT device-side tracing in the future with significantly reduced risk of introducing regressions in the process. Build ===== ompTest is linked against LLVM's GoogleTest by default, but can also be built 'standalone'. Additionally, it comes with a set of unit tests, which in turn require GoogleTest (overriding a standalone build). The unit tests are added to the `check-openmp` target. Use the following parameters to perform the corresponding build: `LIBOMPTEST_BUILD_STANDALONE` (Default: OFF) `LIBOMPTEST_BUILD_UNITTESTS` (Default: OFF) --------- Co-authored-by: Jan-Patrick Lehr <[email protected]>
ab941a7
to
f024da4
Compare
Good catch, thanks for pointing that out. It should be corrected now :) |
Maybe as an additional comment, given that it may not be obvious from the initial message: This is the first part of our efforts to upstream our OMPT device-tracing support from OpenMP offloading. |
Am I right, that the unittests in this PR are to test omptest itself, and not some specific event sequences from the OpenMP runtime library? |
Yes, correct. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few comments.
"Build ompTest's unit tests , requires GoogleTest." OFF) | ||
|
||
# In absence of corresponding OMPT support: exit early | ||
if(NOT ${LIBOMPTARGET_OMPT_SUPPORT}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the right CMake var to key off from?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, also noted by @jprotze.
No, not necessarily.
I think with the extended scope of host & device, we might switch to LIBOMP_OMPT_SUPPORT
as indirectly suggested by Joachim.
WDYT?
Do we need to actively "hide" some device-related functionalities when LIBOMP_OMPT_SUPPORT=ON
and LIBOMPTARGET_OMPT_SUPPORT=OFF
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the question is: can we write assertions that automatically adjust to the set of available callbacks? I.e., if the target callback is not supported, but occurs in a sequence, just ignore such callback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking about this I'd say:
ompTest would try to register the callbacks, which fails, emits a print and continues:
But now that I re-read, you want the written assertions to be ignored in absence of the corresponding callback, correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My idea was, that we should be able to write tests which describe the expected callback sequence. Some callbacks are "optional" and will not be triggered with cmake option LIBOMP_OMPT_OPTIONAL=off
.
I think it makes sense to allow the test to specify the necessary/optional callbacks (as another assert statement). As a sane default, it probably makes sense to use the mandatory/optional from the standard.
But, if we want to run some target tests also with OMP_OFFLOAD=disabled
, the target callbacks should probably be optional (while I'm actually not sure what set_callback
reports in this case. Or when compiling without -fopenmp-target?).
get_filename_component(LLVM_INSTALL_PREFIX "${LLVM_INSTALL_PREFIX}" PATH) | ||
get_filename_component(LLVM_INSTALL_PREFIX "${LLVM_INSTALL_PREFIX}" PATH) | ||
get_filename_component(LLVM_INSTALL_PREFIX "${LLVM_INSTALL_PREFIX}" PATH) | ||
get_filename_component(LLVM_INSTALL_PREFIX "${LLVM_INSTALL_PREFIX}" PATH) | ||
get_filename_component(LLVM_INSTALL_PREFIX "${LLVM_INSTALL_PREFIX}" PATH) | ||
get_filename_component(LLVM_INSTALL_PREFIX "${LLVM_INSTALL_PREFIX}" PATH) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this 6 times, or am I just not seeing the difference?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also pointed out by @jprotze:
Yes, we need this same statement six times; it acts as "one directory level up".
So this acts as ../../../../../../
, which in that case: returns the build-directory root.
Since I was also slightly confused when I saw this in LLVMConfig.cmake
, I will see if there's a better way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just add a comment there explaining what it does :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some comments for the first files
"Build ompTest's unit tests , requires GoogleTest." OFF) | ||
|
||
# In absence of corresponding OMPT support: exit early | ||
if(NOT ${LIBOMPTARGET_OMPT_SUPPORT}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this make sense here? Or should it be LIBOMP_OMPT_SUPPORT
?
get_filename_component(LLVM_INSTALL_PREFIX "${LLVM_INSTALL_PREFIX}" PATH) | ||
get_filename_component(LLVM_INSTALL_PREFIX "${LLVM_INSTALL_PREFIX}" PATH) | ||
get_filename_component(LLVM_INSTALL_PREFIX "${LLVM_INSTALL_PREFIX}" PATH) | ||
get_filename_component(LLVM_INSTALL_PREFIX "${LLVM_INSTALL_PREFIX}" PATH) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicates?
|
||
namespace internal { | ||
// clang-format off | ||
event_class_w_custom_body(AssertionSyncPoint, \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer an expansion of these macros. Will be no fun debugging, if a random statement in the macro segfaults.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Understandable, I have no objection to expand these.
|
||
/// Aliases for enum: ompt_target_data_op_t | ||
constexpr ompt_target_data_op_t ALLOC = ompt_target_data_alloc; | ||
constexpr ompt_target_data_op_t H2D = ompt_target_data_transfer_to_device; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think, h2d and d2h were deprecated and deleted?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, in the meantime (of development) these have been deprecated in OpenMP 6.0.
We use the definitions from omp-tools.h.var
:
https://github.com/llvm/llvm-project/blob/main/openmp/runtime/src/include/omp-tools.h.var#L321-L332
Newly added values, like for example ompt_target_data_transfer = 7
, are still missing there.
(OpenMP spec, chapter 33.35 "OMPT target_data_op Type")
Also, AFAICT H2D, D2H, etc. have been deprecated but are still defined / not deleted (33.35 L18-24):
Additional information
The following instances and associated values of the target_data_op OMPT type are also
defined: ompt_target_data_transfer_to_device, with value 2;
ompt_target_data_transfer_from_device, with value 3;
ompt_target_data_transfer_to_device_async, with value 18; and
ompt_target_data_transfer_from_device, with value 19. These instances have been
deprecated.
const void *CodeptrRA = expectedDefault(void *)); | ||
|
||
static OmptAssertEvent | ||
TargetDataOp(const std::string &Name, const std::string &Group, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are there two of them with different argument orderings?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The first one (without default values) resembles the signature of the OpenMP spec.
Additionally, we wanted to provide an "easy-to-use" CTOR, where you may just provide the OpType
(as Name, Group, and Expected may be handled internally).
Other arguments were reordered as reasonable as possible.
Co-authored-by: Joachim <[email protected]>
Description
OpenMP Tooling Interface Testing Library (ompTest) ompTest is a unit testing framework for testing OpenMP implementations. It offers a simple-to-use framework that allows a tester to check for OMPT events in addition to regular unit testing code, supported by linking against GoogleTest by default. It also facilitates writing concise tests while bridging the semantic gap between the unit under test and the OMPT-event testing.
Background
This library has been developed to provide the means of testing OMPT implementations with reasonable effort. Especially, asynchronous or unordered events are supported and can be verified with ease, which may prove to be challenging with LIT-based tests. Additionally, since the assertions are part of the code being tested, ompTest can reference all corresponding variables during assertion.
Basic Usage
OMPT event assertions are placed before the code, which shall be tested. These assertion can either be provided as one block or interleaved with the test code. There are two types of asserters: (1) sequenced "order-sensitive" and (2) set "unordered" assserters. Once the test is being run, the corresponding events are triggered by the OpenMP runtime and can be observed. Each of these observed events notifies asserters, which then determine if the test should pass or fail.
Example (partial, interleaved)
References
This work has been presented at SC'24 workshops, see: https://ieeexplore.ieee.org/document/10820689
Current State and Future Work
ompTest's development was mostly device-centric and aimed at OMPT device callbacks and device-side tracing. Consequentially, a substantial part of host-related events or features may not be supported in its current state. However, we are confident that the related functionality can be added and ompTest provides a general foundation for future OpenMP and especially OMPT testing. This PR will allow us to upstream the corresponding features, like OMPT device-side tracing in the future with significantly reduced risk of introducing regressions in the process.
Build
ompTest is linked against LLVM's GoogleTest by default, but can also be built 'standalone'. Additionally, it comes with a set of unit tests, which in turn require GoogleTest (overriding a standalone build). The unit tests are added to the
check-openmp
target.Use the following parameters to perform the corresponding build:
LIBOMPTEST_BUILD_STANDALONE
(Default: OFF)LIBOMPTEST_BUILD_UNITTESTS
(Default: OFF)