Reland: [OpenMP] Add ompTest library to OpenMP #154786
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
openmp:libomp
mhalk
requested review from
TerryLWilmarth,
dhruvachak,
hansangbae,
jplehr,
jprotze and
mjklemm
Collaborator
|
My local build still fails during linking openmp/tools/omptest/libomptest.so: |
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);
#pragma omp target parallel for
{
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: ${OPENMP_STANDALONE_BUILD})
`LIBOMPTEST_BUILD_UNITTESTS` (Default: OFF)
---------
Co-authored-by: Jan-Patrick Lehr <[email protected]>
Co-authored-by: Joachim <[email protected]>
Adapted minimum CMake version Fixed llvm_gtest linking - Replaced archive path with target name - Fixed related warning and added checks for llvm_gtest
jprotze
approved these changes
Aug 21, 2025
Collaborator
jprotze
left a comment
jprotze
left a comment
There was a problem hiding this comment.
Lgtm
With the additional dependencies a fresh libc++-first build succeeds.
mhalk
force-pushed
the
mhalk/feat/openmp-omptest-lib-reland
branch
from
aa3bb7b to
880552c
Compare
Contributor
Author
|
Apologies @jprotze I was working on this and blindly force-pushed as I was not aware of your commit. |
Contributor
Author
|
Thank you Joachim, much appreciated! :) |
Collaborator
|
|
nikic
reviewed
Aug 25, 2025
|
|
||
| # Add LLVM-provided GoogleTest include directories. | ||
| target_include_directories(omptest PRIVATE | ||
| ${LLVM_THIRD_PARTY_DIR}/unittest/googletest/include) |
Contributor
There was a problem hiding this comment.
Why is this needed? Shouldn't this be covered on the PUBLIC target_include_directories on llvm_gtest?
Contributor
Author
There was a problem hiding this comment.
Yes, I think you're right. AFAICT this can be removed.
|
|
||
| # TODO: Re-visit ABI breaking checks, disable for now. | ||
| target_compile_definitions(omptest PUBLIC | ||
| -DLLVM_DISABLE_ABI_BREAKING_CHECKS_ENFORCING) |
Contributor
There was a problem hiding this comment.
Why is this needed? This looks very fishy.
Contributor
Author
There was a problem hiding this comment.
Actually I had build failures without it, but this seems to depend on how you link LLVMSupport.
It seems to be implicitly linked as PRIVATE but needs PUBLIC from what I found out during tests.
|
|
||
| # Link llvm_gtest as whole-archive to expose required symbols | ||
| set(GTEST_LINK_CMD "-Wl,--whole-archive" llvm_gtest | ||
| "-Wl,--no-whole-archive" LLVMSupport) |
Contributor
There was a problem hiding this comment.
Do you really need this now that you're using the llvm_gtest target? None of the other places that use llvm_gtest require this. Why does this one?
Contributor
Author
There was a problem hiding this comment.
Actually "yes", ompTest should be usable like an OMPT-extended version of GoogleTest.
Thus, my goal is pulling in all symbols, but I'm by no means an expert on this matter.
The issue is that without it, there are linker errors when a user-test links against libomptest.so.
That's because of missing symbols, like e.g. testing::Test::Test() or testing::internal::GTestLog::~GTestLog().
Advice on how to improve this part would be greatly appreciated.
One possibility I saw was to use an intermediate build file, similar to this:
[...]
set(GTEST_OBJECT "build/third-party/unittest/CMakeFiles/llvm_gtest.dir/googletest/src/gtest-all.cc.o")
# Add GoogleTest-based header and merge pre-built object
target_sources(omptest PRIVATE
./include/OmptTesterGoogleTest.h
"${GTEST_OBJECT}")
[...]Apart from locating that file properly, I'm not sure which other dependencies this approach would imply (maybe compiler-rt?).
Member
|
I'm seeing build failures with this change, while using runtimes build: and similar. Configured via: The file's present, so I guess it's a missing include directory: |
Collaborator
|
The other tools have this line in their CMake config: |
Member
|
@mhalk, ping. |
mgorny
added a commit
to mgorny/llvm-project
that referenced
this pull request
Aug 30, 2025
Add missing `LIBOMP_INCLUDE_DIR` include directory to fix build failures in omptest, as reported in llvm#154786 (comment). Thanks fo @jprotze for the suggested fix. Signed-off-by: Michał Górny <[email protected]>
mgorny
added a commit
that referenced
this pull request
Aug 31, 2025
Add missing `LIBOMP_INCLUDE_DIR` include directory to fix build failures in omptest, as reported in #154786 (comment). Thanks fo @jprotze for the suggested fix. Signed-off-by: Michał Górny <[email protected]>
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Aug 31, 2025
…56194) Add missing `LIBOMP_INCLUDE_DIR` include directory to fix build failures in omptest, as reported in llvm/llvm-project#154786 (comment). Thanks fo @jprotze for the suggested fix. Signed-off-by: Michał Górny <[email protected]>
Contributor
Author
|
Apologies for the radio-silence, I'll re-check all comments -- thanks for the input. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reland of #147381
Added changes to fix observed BuildBot failures:
3.20, was:3.22)./build/lib/libllvm_gtest.a)#include "llvm/Support/raw_os_ostream.h")