Explicitly link libdl as suggested by @vzakhari.

Fix the special case handling for RunningOnValgrind pointed out by @Hahnfeld . We don't want to have output for a missing call, so it still has a separate macro.

Since you seem to have access to a Mac, can you at least paste the output?

I'm posting here, because this change broke LLVM tests on a machine only supporting SSE3.

Why do we need __kmpc_omp_taskwait_51 ? Taskwait nowait only makes sense with dependencies. It should only result in additional nodes/edges in the task dependency graph without any code to execute. There is not taskwait (aka task barrier) in this case.

lgtm

I did not argue against adding ((char *)addr) - 12. But following the description in the comment, I think, ((char *)addr) - 4 should stay there, as this might still be valid for other cases (e.g., compiling with other optimization constraints?).

Your change only allows offsets of 8 and 12 byte. But the comment indicates that 4 byte offset might still be possible? I think, you should just print the 12 byte offset in addition.

LGTM

lgtm

This change bricks the old tsan runtime. Should we remove the call from rtl-old as well? Or is rtl-old not maintained at all?

Lgtm

Without testing, I would say the variable is not visible in this CMake scope, as it is defined in openmp/runtime/src/CMakeLists.txt. I would assume that you need to set it with PARENT_SCOPE in openmp/runtime/src/CMakeLists.txt and openmp/runtime/CMakeLists.txt. Similar to LIBOMP_LIBRARY_DIR or LIBOMP_INCLUDE_DIR.

Lgtm

An alternative approach would be to install the python modules wheels and setuptools into the build directory using pip instead of bailing out.

Just one more change. Then the ompt part looks good to me.

In D128896#3623818, @tianshilei1992 wrote:

I don't understand. Here we all do >> operation. How could it cause overflow?

Are the llvm version variables defined for stand-alone builds? So, will stand-alone builds of OpenMP continue to work and include libomptarget code?

lgtm now

Thanks for the fix! I'm quite confident that this change will fix the sporiadic test failures (most probably under high system load).

This patch LGTM.

I checked the runtime tests and found that openmp/runtime/test/barrier/omp_barrier.c already triggers different combinations of barriers. Do the combinations cover all configurations you tried manually?
Regarding reductions, I think it would make sense to add similar run lines to openmp/runtime/test/worksharing/for/omp_for_reduction.c in order to cover different code paths in the runtime.

I agree, the expected output does not fit the general case of reductions.
As a workaround, I would change the check prefix for reduction 2-n from CHECK to MULTIPLE and add --check-prefixes CHECK,MULTIPLE for the RUN-lines, where multiple pairs of reduction callbacks are expected. So the default would be "at least one" pair of callbacks.

I think, in addition to manually selecting the barrier implementation, we should add additional run lines to specific tests that trigger the different barrier implementations. I added a related, OMPT-specific comment to D123193.

Can you add extra run lines to openmp/runtime/test/ompt/synchronization/reduction/*_reduce.c in order to trigger the different barrier implementations?

Thanks for working on this!

I think, the name of the variable is confusing. The tests triggered and influenced by your example command are not limited to libomp. Currently the LIBOMP_ prefix is mainly used for cmake variables. The make target to run the tests is called check-* rather than test-*.

For the test I suggest signalling like used for OMPT tests (see openmp/runtime/tests/ompt/ompt_signal.h) to enforce the execution ordering rather than sleep "synchronization".

In D119988#3341375, @tianshilei1992 wrote:

I change it back to 256.

I think in openmp/tools currently only libraries and header files get installed, but no binaries.

Thanks for the update. LGTM.
Can you commit yourself, or should I commit the fix for you?

Just return, if (Data->Parent == nullptr).

lgtm

I guess the question is, whether we expect more reserved locators will be introduced in the future. Since the spec actually introduced a new section, I thought this might really happen. During OMPT discussion, Deepak also raised the issue, that these future reserved locators might not be limited to inout/out dependencies.
Therefore, I thought it would make sense to let the flag just indicate that this involves a reserved locator and encode the kind of locator in the address argument.

In D114005#3135157, @dreachem wrote:

What happens for a teams construct? The implicit tasks in that case are all initial tasks. Should this Data->Team->Delete() apply there as well, or only for the implicit parallel region for the whole program?

I mainly checked the libomp code and put some inline comments.

Thanks for the clarification.
lgtm

@jlpeyton can you check with your compiler team, what is wrong with icc 2021? icx works fine.

Tests should not burn CPU cycles. We use my_sleep from omp_my_sleep.h or delay from ompt-signal.h instead to allow other tests make progress in the meantime.

Looks like we missed to update the code in kmp_csupport, when the management of lwt was refactored.
lgtm

In my local test execution I just got:

lgtm

Writing up the OMPT interface for omp_all_memory, I realized that treating omp_all_memory individually might not be the best choice, considering that the spec talks about "Reserved Locators".

We will finish and discuss the spec ticket during the OMPT call next week. I'll commit after the call - just in case we decide for different names.

Why are the types in __kmpc_atomic_val_4_cas_cpt different from the types in __kmpc_atomic_val_4_cas? I think, all the functions should use fixed-size integer to respect the width suggested by the name of the function.

Please update the diff with context (-U9999)

Please update the diff with context (-U9999)

I already know, that OMPD folks will not be happy about this implementation. Nevertheless, it is a valid and better implementation than before.

Overall looks good to me besides one inline comment.
I successfully tested with gcc/11, icc/19 and latest clang.

lgtm