Page MenuHomePhabricator

[OpenMP] Fix builds for non-x86 after distributed barrier patch
Needs ReviewPublic

Authored by jlpeyton on Jun 23 2021, 8:10 AM.

Details

Summary

The new distributed barrier patch needs basic support for non-x86 platforms.
This patch adds non-x86 versions of KMP_M|SFENCE() as aliases to KMP_MB()
and a more robust configure-time detection of aligned memory allocator. If no aligned
memory allocator is found, then just use plain old malloc since the alignment isn't
necessary for functionality.

Diff Detail

Unit TestsFailed

TimeTest
790 msx64 debian > libomp.lock::omp_init_lock.c
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang -fopenmp -pthread -fno-experimental-isel -I /var/lib/buildkite-agent/builds/llvm-project/openmp/runtime/test -I /var/lib/buildkite-agent/builds/llvm-project/build/projects/openmp/runtime/src -L /var/lib/buildkite-agent/builds/llvm-project/build/lib -I /var/lib/buildkite-agent/builds/llvm-project/openmp/runtime/test/ompt /var/lib/buildkite-agent/builds/llvm-project/openmp/runtime/test/lock/omp_init_lock.c -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/openmp/runtime/test/lock/Output/omp_init_lock.c.tmp -lm -latomic && /var/lib/buildkite-agent/builds/llvm-project/build/projects/openmp/runtime/test/lock/Output/omp_init_lock.c.tmp

Event Timeline

jlpeyton created this revision.Jun 23 2021, 8:10 AM
jlpeyton requested review of this revision.Jun 23 2021, 8:10 AM
jdenny added a subscriber: jdenny.Jun 23 2021, 1:01 PM
jlpeyton updated this revision to Diff 354258.Jun 24 2021, 8:07 AM

Enable error detecting failed memory allocation for any of the aligned memory allocators.

ggeorgakoudis added a subscriber: ggeorgakoudis.EditedJun 25 2021, 4:10 PM

I had the same compilation errors mentioned in D103121 on an IBM machine. I can confirm that with this patch errors go away and compilation finishes. However, there are several warnings on overriden functions and virtual destructors, for example:

.../llvm-project/openmp/runtime/src/kmp_wait_release.h:958:8: warning: 'notdone_check' overrides a member
 function but is not marked 'override' [-Wsuggest-override]                                                                               
  bool notdone_check() {                                                                                                                  
       ^                                                                                                                                  
.../llvm-project/openmp/runtime/src/kmp_wait_release.h:178:16: note: overridden virtual function is here 
  virtual bool notdone_check() {

and

.../llvm-project/openmp/runtime/src/kmp.h:266:48: warning: 'kmp_flag_64<false, true>' has virtual functio
ns but non-virtual destructor [-Wnon-virtual-dtor]                                                                                        
template <bool C = false, bool S = true> class kmp_flag_64;                                                                               
                                               ^

which should be reasonable to fix too. Happy to provide a more detailed log if needed.

I had the same compilation errors mentioned in D103121 on an IBM machine. I can confirm that with this patch errors go away and compilation finishes. However, there are several warnings on overriden functions and virtual destructors, for example:

.../llvm-project/openmp/runtime/src/kmp_wait_release.h:958:8: warning: 'notdone_check' overrides a member
 function but is not marked 'override' [-Wsuggest-override]                                                                               
  bool notdone_check() {                                                                                                                  
       ^                                                                                                                                  
.../llvm-project/openmp/runtime/src/kmp_wait_release.h:178:16: note: overridden virtual function is here 
  virtual bool notdone_check() {

and

.../llvm-project/openmp/runtime/src/kmp.h:266:48: warning: 'kmp_flag_64<false, true>' has virtual functio
ns but non-virtual destructor [-Wnon-virtual-dtor]                                                                                        
template <bool C = false, bool S = true> class kmp_flag_64;                                                                               
                                               ^

which should be reasonable to fix too. Happy to provide a more detailed log if needed.

A detailed log would help!

I had the same compilation errors mentioned in D103121 on an IBM machine. I can confirm that with this patch errors go away and compilation finishes. However, there are several warnings on overriden functions and virtual destructors, for example:

.../llvm-project/openmp/runtime/src/kmp_wait_release.h:958:8: warning: 'notdone_check' overrides a member
 function but is not marked 'override' [-Wsuggest-override]                                                                               
  bool notdone_check() {                                                                                                                  
       ^                                                                                                                                  
.../llvm-project/openmp/runtime/src/kmp_wait_release.h:178:16: note: overridden virtual function is here 
  virtual bool notdone_check() {

and

.../llvm-project/openmp/runtime/src/kmp.h:266:48: warning: 'kmp_flag_64<false, true>' has virtual functio
ns but non-virtual destructor [-Wnon-virtual-dtor]                                                                                        
template <bool C = false, bool S = true> class kmp_flag_64;                                                                               
                                               ^

which should be reasonable to fix too. Happy to provide a more detailed log if needed.

A detailed log would help!

Probably true.

I'll go ahead and revert the original patch so we can sort this out properly.
I was under the impression that the issue was going to be addressed right away but the fix seems to take longer.
It is not in anyone interest that our build is broken on non-X86 machines for 10 days now, arguably a revert right away would have
been the right course of action.

I had the same compilation errors mentioned in D103121 on an IBM machine. I can confirm that with this patch errors go away and compilation finishes. However, there are several warnings on overriden functions and virtual destructors, for example:

.../llvm-project/openmp/runtime/src/kmp_wait_release.h:958:8: warning: 'notdone_check' overrides a member
 function but is not marked 'override' [-Wsuggest-override]                                                                               
  bool notdone_check() {                                                                                                                  
       ^                                                                                                                                  
.../llvm-project/openmp/runtime/src/kmp_wait_release.h:178:16: note: overridden virtual function is here 
  virtual bool notdone_check() {

and

.../llvm-project/openmp/runtime/src/kmp.h:266:48: warning: 'kmp_flag_64<false, true>' has virtual functio
ns but non-virtual destructor [-Wnon-virtual-dtor]                                                                                        
template <bool C = false, bool S = true> class kmp_flag_64;                                                                               
                                               ^

which should be reasonable to fix too. Happy to provide a more detailed log if needed.

A detailed log would help!

Here is the detailed log: https://pastebin.com/pNhdtwH8