This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP] libomp: Add new experimental barrier: two-level distributed barrier
ClosedPublic

Authored by tlwilmar on May 25 2021, 3:22 PM.

Details

Summary

Two-level distributed barrier is a new experimental barrier designed
for Intel hardware that has better performance in some cases than the
default hyper barrier.

This barrier is designed to handle fine granularity parallelism where
barriers are used frequently with little compute and memory access
between barriers. There is no need to use it for codes with few
barriers and large granularity compute, or memory intensive
applications, as little difference will be seen between this barrier
and the default hyper barrier. This barrier is designed to work
optimally with a fixed number of threads, and has a significant setup
time, so should NOT be used in situations where the number of threads
in a team is varied frequently.

The two-level distributed barrier is off by default -- hyper barrier
is used by default. To use this barrier, you must set all barrier
patterns to use this type, because it will not work with other barrier
patterns. Thus, to turn it on, the following settings are required:

KMP_FORKJOIN_BARRIER_PATTERN=dist,dist
KMP_PLAIN_BARRIER_PATTERN=dist,dist
KMP_REDUCTION_BARRIER_PATTERN=dist,dist

Branching factors (set with KMP_FORKJOIN_BARRIER, KMP_PLAIN_BARRIER,
and KMP_REDUCTION_BARRIER) are ignored by the two-level distributed
barrier.

Diff Detail

Event Timeline

tlwilmar created this revision.May 25 2021, 3:22 PM
tlwilmar requested review of this revision.May 25 2021, 3:22 PM

Can you add the lit run lines to test this new barrier in test/barrier/omp_barrier.c?

tlwilmar updated this revision to Diff 352140.Jun 15 2021, 8:03 AM

Added test lines.

jlpeyton accepted this revision.Jun 16 2021, 8:29 AM

Thanks! LGTM

This revision is now accepted and ready to land.Jun 16 2021, 8:29 AM
This revision was landed with ongoing or failed builds.Jun 16 2021, 1:35 PM
This revision was automatically updated to reflect the committed changes.
kkwli0 added a subscriber: kkwli0.Jun 18 2021, 4:04 PM

Building on Power running RHEL has the following errors.

/llvm-project/openmp/runtime/src/kmp_barrier.cpp:371:3: error: use of undeclared identifier 'KMP_MFENCE'
  KMP_MFENCE();
  ^
/llvm-project/openmp/runtime/src/kmp_barrier.h:82:51: error: use of undeclared identifier '_mm_malloc'
    distributedBarrier *d = (distributedBarrier *)_mm_malloc(
                                                  ^

Building on Power running RHEL has the following errors.

/llvm-project/openmp/runtime/src/kmp_barrier.cpp:371:3: error: use of undeclared identifier 'KMP_MFENCE'
  KMP_MFENCE();
  ^
/llvm-project/openmp/runtime/src/kmp_barrier.h:82:51: error: use of undeclared identifier '_mm_malloc'
    distributedBarrier *d = (distributedBarrier *)_mm_malloc(
                                                  ^

Kelvin, I've created https://reviews.llvm.org/D104788 to fix this, can you check this patch and see if it fixes your problem.

jlpeyton reopened this revision.Jul 9 2021, 1:02 PM
This revision is now accepted and ready to land.Jul 9 2021, 1:02 PM
tlwilmar updated this revision to Diff 357929.Jul 12 2021, 6:37 AM

This update contains the original patch plus https://reviews.llvm.org/D104638, https://reviews.llvm.org/D104788, and a few more additions to fix the warnings mentioned in D104788.

I think the original approval is still good, assuming this doesn't break other targets. Maybe have someone test it for you.

tlwilmar updated this revision to Diff 358016.Jul 12 2021, 11:52 AM

kmp_barrier.h was missing; added back.

Is the diff from HEAD? I try to build it but the patch cannot be applied to the trunk.

tlwilmar updated this revision to Diff 358312.Jul 13 2021, 9:41 AM

Kelvin, please try again with this new patch. As of this moment, at least, it applies on top of main branch.

kkwli0 added a comment.EditedJul 13 2021, 10:03 PM

Thanks Terry. Unfortunately, I got the following errors.

CMakeFiles/omp.dir/kmp_runtime.cpp.o: In function `~kmp_flag_64':
llvm-project/openmp/runtime/src/kmp.h:266: undefined reference to `operator delete(void*)'
CMakeFiles/omp.dir/kmp_runtime.cpp.o: In function `~kmp_flag_native':
llvm-project/openmp/runtime/src/kmp_wait_release.h:157: undefined reference to `operator delete(void*)'
CMakeFiles/omp.dir/kmp_barrier.cpp.o: In function `~kmp_flag_oncore':
llvm-project/openmp/runtime/src/kmp_wait_release.h:955: undefined reference to `operator delete(void*)'
CMakeFiles/omp.dir/kmp_barrier.cpp.o: In function `~kmp_flag_native':
llvm-project/openmp/runtime/src/kmp_wait_release.h:157: undefined reference to `operator delete(void*)'
CMakeFiles/omp.dir/kmp_barrier.cpp.o: In function `~kmp_flag_64':
/llvm-project/openmp/runtime/src/kmp_wait_release.h:855: undefined reference to `operator delete(void*)'
CMakeFiles/omp.dir/kmp_barrier.cpp.o:llvm-project/openmp/runtime/src/kmp_wait_release.h:157: more undefined references to `operator delete(void*)' follow
tlwilmar updated this revision to Diff 359089.Jul 15 2021, 12:17 PM

Patch notes from Jonathan Peyton: The -Wl,-z,defs flag sent in from LLVM cause undefined references which cause link-time errors in object files due to the new empty virtual destructors. We eliminate this flag for the host OpenMP runtime which eliminates the undefined reference errors. This prevents the host runtime from requiring linking to the standard C++ runtime library.

@tlwilmar Is kmp_barrier.h accidentally removed in the latest update?

The -Wl,-z,defs flag sent in from LLVM cause undefined references which cause link-time errors in object files

This flag does not cause or create undefined references. It warns when there are some undefined references. Better to fix the missing symbols than to hope the application never tries to call them.

tlwilmar updated this revision to Diff 360491.Jul 21 2021, 9:04 AM

Johnny added overloaded operator new() and operator delete() functions to the two kmp_flag classes that have virtual destructors. He took out the removal of the -Wl,-z,defs flag from the previous patch as well.
He applied it to LLVM and built all of LLVM using clang-12 (with no warnings now and no undefined references), then ran check-openmp and everything was OK.
Kelvin, please wait for the x86 build bot to give a passing result, before attempting to test. Thanks!

tlwilmar requested review of this revision.Jul 23 2021, 11:43 AM

@kkwli0 Other than some clang-format and clang-tidy issues (ignorable), builds look okay, so I think this is ready to try out.

@tlwilmar It builds successfully on Power9 running RHEL 8.2. Thanks.

This revision was not accepted when it landed; it landed in state Needs Review.Jul 29 2021, 12:09 PM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.