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

[jlpeyton]

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.

[jlpeyton]

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

[ggeorgakoudis]

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.

[jlpeyton]

In D104788#2841928, @ggeorgakoudis wrote:

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!

[jdoerfert]

In D104788#2847080, @jlpeyton wrote:

In D104788#2841928, @ggeorgakoudis wrote:

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.

[ggeorgakoudis]

In D104788#2847080, @jlpeyton wrote:

In D104788#2841928, @ggeorgakoudis wrote:

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

[jlpeyton]

This patch is no longer relevant.

openmp/runtime/cmake/config-ix.cmake

 # Check for Unix shared memory
 check_symbol_exists(shm_open "sys/mman.h" LIBOMP_HAVE_SHM_OPEN_NO_LRT)
 if (NOT LIBOMP_HAVE_SHM_OPEN_NO_LRT)
   set(CMAKE_REQUIRED_LIBRARIES -lrt)
   check_symbol_exists(shm_open "sys/mman.h" LIBOMP_HAVE_SHM_OPEN_WITH_LRT)
   set(CMAKE_REQUIRED_LIBRARIES)
 endif()
 
+# Check for aligned memory allocator function
+check_include_file(xmmintrin.h LIBOMP_HAVE_XMMINTRIN_H)
+set(OLD_CMAKE_REQUIRED_FLAGS ${CMAKE_REQUIRED_FLAGS})
+if (LIBOMP_HAVE_XMMINTRIN_H)
+  set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -DLIBOMP_HAVE_XMMINTRIN_H")
+endif()
+set(source_code "// check for _mm_malloc
+    #ifdef LIBOMP_HAVE_XMMINTRIN_H
+    #include <xmmintrin.h>
+    #endif
+    int main() { void *ptr = _mm_malloc(sizeof(int) * 1000, 64); _mm_free(ptr); return 0; }")
+check_cxx_source_compiles("${source_code}" LIBOMP_HAVE__MM_MALLOC)
+set(CMAKE_REQUIRED_FLAGS ${OLD_CMAKE_REQUIRED_FLAGS})
+check_symbol_exists(aligned_alloc "stdlib.h" LIBOMP_HAVE_ALIGNED_ALLOC)
+check_symbol_exists(posix_memalign "stdlib.h" LIBOMP_HAVE_POSIX_MEMALIGN)
+check_symbol_exists(_aligned_malloc "malloc.h" LIBOMP_HAVE__ALIGNED_MALLOC)
+
 # Check linker flags
 if(WIN32)
   libomp_check_linker_flag(/SAFESEH LIBOMP_HAVE_SAFESEH_FLAG)
 elseif(NOT APPLE)
   libomp_check_linker_flag(-Wl,-x LIBOMP_HAVE_X_FLAG)
   libomp_check_linker_flag(-Wl,--warn-shared-textrel LIBOMP_HAVE_WARN_SHARED_TEXTREL_FLAG)
   libomp_check_linker_flag(-Wl,--as-needed LIBOMP_HAVE_AS_NEEDED_FLAG)
   libomp_check_linker_flag("-Wl,--version-script=${LIBOMP_SRC_DIR}/exports_so.txt" LIBOMP_HAVE_VERSION_SCRIPT_FLAG)

openmp/runtime/src/kmp_barrier.h

 //
 //===----------------------------------------------------------------------===//
 
 #ifndef KMP_BARRIER_H
 #define KMP_BARRIER_H
 
 #include "kmp.h"
 
+#if KMP_HAVE_XMMINTRIN_H && KMP_HAVE__MM_MALLOC
+#include <xmmintrin.h>
+#define KMP_ALIGNED_ALLOCATE(size, alignment) _mm_malloc(size, alignment)
+#define KMP_ALIGNED_FREE(ptr) _mm_free(ptr)
+#elif KMP_HAVE_ALIGNED_ALLOC
+#define KMP_ALIGNED_ALLOCATE(size, alignment) aligned_alloc(alignment, size)
+#define KMP_ALIGNED_FREE(ptr) free(ptr)
+#elif KMP_HAVE_POSIX_MEMALIGN
+#include "kmp_i18n.h"
+static inline void *KMP_ALIGNED_ALLOCATE(size_t size, size_t alignment) {
+  void *ptr;
+  int n = posix_memalign(&ptr, alignment, size);
+  if (n != 0) {
+    __kmp_fatal(KMP_MSG(FunctionError, "posix_memalign()"), KMP_ERR(errno),
+                __kmp_msg_null);
+  }
+  return ptr;
+}
+#define KMP_ALIGNED_FREE(ptr) free(ptr)
+#elif KMP_HAVE__ALIGNED_MALLOC
+#include <malloc.h>
+#define KMP_ALIGNED_ALLOCATE(size, alignment) _aligned_malloc(size, alignment)
+#define KMP_ALIGNED_FREE(ptr) _aligned_free(ptr)
+#else
+#define KMP_ALIGNED_ALLOCATE(size, alignment) KMP_INTERNAL_MALLOC(size)
+#define KMP_ALIGNED_FREE(ptr) KMP_INTERNAL_FREE(ptr)
+#endif
+
 // Use four cache lines: MLC tends to prefetch the next or previous cache line
 // creating a possible fake conflict between cores, so this is the only way to
 // guarantee that no such prefetch can happen.
 #ifndef KMP_FOURLINE_ALIGN_CACHE
 #define KMP_FOURLINE_ALIGN_CACHE KMP_ALIGN(4 * CACHE_LINE)
 #endif
 
 #define KMP_OPTIMIZE_FOR_REDUCTIONS 0
public:
   size_t KMP_ALIGN_CACHE gos_per_group;
   void *team_icvs;
 
   distributedBarrier() = delete;
   ~distributedBarrier() = delete;
 
   // Used instead of constructor to create aligned data
   static distributedBarrier *allocate(int nThreads) {
-    distributedBarrier *d = (distributedBarrier *)_mm_malloc(
+    distributedBarrier *d = (distributedBarrier *)KMP_ALIGNED_ALLOCATE(
         sizeof(distributedBarrier), 4 * CACHE_LINE);
     d->num_threads = 0;
     d->max_threads = 0;
     for (int i = 0; i < MAX_ITERS; ++i)
       d->flags[i] = NULL;
     d->go = NULL;
     d->iter = NULL;
     d->sleep = NULL;
     d->team_icvs = NULL;
     d->fix_threads_per_go = false;
     // calculate gos and groups ONCE on base size
     d->computeGo(nThreads);
     d->init(nThreads);
     return d;
   }
 
-  static void deallocate(distributedBarrier *db) { _mm_free(db); }
+  static void deallocate(distributedBarrier *db) { KMP_ALIGNED_FREE(db); }
 
   void update_num_threads(size_t nthr) { init(nthr); }
 
   bool need_resize(size_t new_nthr) { return (new_nthr > max_threads); }
   size_t get_num_threads() { return num_threads; }
   kmp_uint64 go_release();
   void go_reset();
 };
 
 #endif // KMP_BARRIER_H

openmp/runtime/src/kmp_config.h.cmake

 #cmakedefine01 LIBOMP_HAVE_INTRIN_H
 #define KMP_HAVE_INTRIN_H LIBOMP_HAVE_INTRIN_H
 #cmakedefine01 LIBOMP_HAVE_ATTRIBUTE_WAITPKG
 #define KMP_HAVE_ATTRIBUTE_WAITPKG LIBOMP_HAVE_ATTRIBUTE_WAITPKG
 #cmakedefine01 LIBOMP_HAVE_ATTRIBUTE_RTM
 #define KMP_HAVE_ATTRIBUTE_RTM LIBOMP_HAVE_ATTRIBUTE_RTM
 #cmakedefine01 LIBOMP_ARCH_AARCH64_A64FX
 #define KMP_ARCH_AARCH64_A64FX LIBOMP_ARCH_AARCH64_A64FX
+#cmakedefine01 LIBOMP_HAVE_XMMINTRIN_H
+#define KMP_HAVE_XMMINTRIN_H LIBOMP_HAVE_XMMINTRIN_H
+#cmakedefine01 LIBOMP_HAVE__MM_MALLOC
+#define KMP_HAVE__MM_MALLOC LIBOMP_HAVE__MM_MALLOC
+#cmakedefine01 LIBOMP_HAVE_ALIGNED_ALLOC
+#define KMP_HAVE_ALIGNED_ALLOC LIBOMP_HAVE_ALIGNED_ALLOC
+#cmakedefine01 LIBOMP_HAVE_POSIX_MEMALIGN
+#define KMP_HAVE_POSIX_MEMALIGN LIBOMP_HAVE_POSIX_MEMALIGN
+#cmakedefine01 LIBOMP_HAVE__ALIGNED_MALLOC
+#define KMP_HAVE__ALIGNED_MALLOC LIBOMP_HAVE__ALIGNED_MALLOC
 
 // Configured cache line based on architecture
 #if KMP_ARCH_PPC64
 # define CACHE_LINE 128
 #elif KMP_ARCH_AARCH64_A64FX
 # define CACHE_LINE 256
 #else
 # define CACHE_LINE 64

openmp/runtime/src/kmp_os.h

 #define KMP_MFENCE()                                                           \
   if (UNLIKELY(!__kmp_cpuinfo.initialized)) {                                  \
     __kmp_query_cpuid(&__kmp_cpuinfo);                                         \
   }                                                                            \
   if (__kmp_cpuinfo.sse2) {                                                    \
     KMP_MFENCE_();                                                             \
   }
 #define KMP_SFENCE() KMP_SFENCE_()
+#else
+#define KMP_MFENCE() KMP_MB()
+#define KMP_SFENCE() KMP_MB()
 #endif
 
 #ifndef KMP_IMB
 #define KMP_IMB() /* nothing to do */
 #endif
 
 #ifndef KMP_ST_REL32
 #define KMP_ST_REL32(A, D) (*(A) = (D))