Test whether OMPT-callbacks for two threads that initiate a parallel region are correct.
Details
Diff Detail
Event Timeline
runtime/test/ompt/misc/interoperability.cpp | ||
---|---|---|
1 | I think, this is not needed for this case. Just run the sorted checks.
|
runtime/test/ompt/misc/interoperability.cpp | ||
---|---|---|
40 | These three checks relates to the second master thread, so to make it clear I'd suggest naming it MASTER_ID_2. | |
47 | Two checks above should has different thread ids, that is THREAD_ID_1, THREAD_ID_2 (something like that). I also suggest adding CHECK for *_end events for all *_begin events above. |
I think if you re-capture a variable with FileCheck that's the same as capturing another variable except that you can't use the old value. So I'm not sure if the changes make any difference, besides naming...
Yes, I understand that. But it is difficult to understand what the test is actually checking due to no comments and same identifiers usage.
Implemented changes look good.
However, it's probably worth to apply re-formatting suggested by @Hahnfeld in https://reviews.llvm.org/D43191.
Could you please reformat the full file with clang-format? The indention doesn't look right...
I applied clang-format on commit.
Having the tests after the end of the main function gives us actually 2 more characters :)
This test doesn't work if threads are not scheduled as expected: http://lab.llvm.org:8011/builders/openmp-gcc-x86_64-linux-debian/builds/58
I think in that case a worker thread started before the second master which makes sense as there is no synchronization between the two initial threads. We should be able to fix this by either starting a serialized region or calling an API function and putting an OMPT_WAIT before the real parallel region. Thoughts?
Btw: What do you need the OMPT_WAIT(condition, 6) for?
The idea was that the WAIT in line 11 should ensure that both initial threads arrived.
But actually the runtime is not initialized before line 11. To fix the race, we need to call into the runtime in a way, that makes both threads initial threads before the SIGNAL in line 9.
The WAIT in line 16 is to ensure that both worker threads are recruited separately. Not sure whether it would be possible that the same thread is recruited by both initial threads.
I think, this is not needed for this case. Just run the sorted checks.