[OpenMP] libomp: add atomic functions for new OpenMP 5.1 atomics.
ClosedPublic
Actions

Authored by AndreyChurbanov on Sep 20 2021, 2:51 PM.

Details

Reviewers

hbae
jlpeyton
tlwilmar
Nawrin
jdoerfert

Commits

rG621d7a75b16d: [OpenMP] libomp: add atomic functions for new OpenMP 5.1 atomics.

Summary

Added functions those implement "atomic compare".
Though clang does not use library interfaces to implement OpenMP atomics,
the functions added for consistency.

Also added missed functions for 80-bit floating min/max atomics.

Diff Detail

Repository: rG LLVM Github Monorepo

Event Timeline

AndreyChurbanov created this revision.Sep 20 2021, 2:51 PM

Herald added subscribers: guansong, yaxunl. · View Herald TranscriptSep 20 2021, 2:51 PM

AndreyChurbanov requested review of this revision.Sep 20 2021, 2:51 PM

Herald added a reviewer: jdoerfert. · View Herald TranscriptSep 20 2021, 2:51 PM

Herald added subscribers: openmp-commits, sstefan1. · View Herald Transcript

Harbormaster completed remote builds in B124762: Diff 373717.Sep 20 2021, 3:23 PM

LGTM

This revision is now accepted and ready to land.Oct 13 2021, 10:17 AM

Closed by commit rG621d7a75b16d: [OpenMP] libomp: add atomic functions for new OpenMP 5.1 atomics. (authored by AndreyChurbanov). · Explain WhyOct 13 2021, 11:02 AM

This revision was automatically updated to reflect the committed changes.

AndreyChurbanov added a commit: rG621d7a75b16d: [OpenMP] libomp: add atomic functions for new OpenMP 5.1 atomics..

Is this patch only applicable to KMP_ARCH_X86 || KMP_ARCH_X86_64 arch? The newly added test cases fail on non-X86 platform due to undefined symbols. The __kmpc_atomic_*_cas_* prototypes got preprocessed out inside the extern "C" hence the names are mangled.

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.

AndreyChurbanov mentioned this in D112261: [OpenMP] libomp: disable definitions of 5.1 atomics for non-x86 arch.Oct 21 2021, 12:20 PM

In D110109#3077855, @kkwli0 wrote:

Is this patch only applicable to KMP_ARCH_X86 || KMP_ARCH_X86_64 arch? The newly added test cases fail on non-X86 platform due to undefined symbols. The __kmpc_atomic_*_cas_* prototypes got preprocessed out inside the extern "C" hence the names are mangled.

Thanks for comment. The implementation is indeed supposed to be applicable to KMP_ARCH_X86 || KMP_ARCH_X86_64 only.
I've fixed the tests in commit https://reviews.llvm.org/rG52f4922ebb7b. Library code fixed in https://reviews.llvm.org/D112261 to eliminate mangled symbols from the build on non-x86 architectures.

In D110109#3077907, @protze.joachim wrote:

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.

Sorry, Joachim, I haven't got your comment. Both functions deal with kmp_int32 that is 4 bytes. Or am I missing something?

@AndreyChurbanov There is one more issue. kmp_atomic_float10_max_min.c specifies -mlong-double-80 but some platforms (e.g. Power) do not support such option.

In D110109#3089150, @kkwli0 wrote:

@AndreyChurbanov There is one more issue. kmp_atomic_float10_max_min.c specifies -mlong-double-80 but some platforms (e.g. Power) do not support such option.

Thanks for reporting.
I've added "UNSUPPORTED: powerpc" to the test in git commit 6f0125eec75f, hope it fixes the problem.

alvinhochun mentioned this in D137168: [OpenMP][mingw] Fix build for aarch64 target.Nov 2 2022, 12:58 AM