[openmp] Support building for armv7 Windows with mingw tools

[mstorsjo]

Summary

This does things in the same way as
D137168 / a356782426f5bf54a00570e1f925345e5fda7b2e and
D101173 / 4fb0aaf03381473ec8af727edb4b5d59b64b0d60 did for aarch64.

This adds a C implementation of __kmp_invoke_microtask in the same
way as the fallback C implementation in z_Linux_util.cpp - however
this one handles up to 17 arguments, which makes
test/misc_bugs/many-microtask-args.c pass. (The version in
z_Linux_util.cpp only handles up to 15 arguments. And contrary to
aarch64, z_Linux_asm.S doesn't have any assembly for 32 bit arm.)

This passes all tests (with D137748 and D137772 applied).

[DavidSpickett]

(The version in z_Linux_util.cpp only handles up to 15 arguments. And contrary to aarch64, z_Linux_asm.S doesn't have any assembly for 32 bit arm.)

So this means that simply extending it to handle 17 arguments wouldn't help because it would want to use the assembly version of the other functions?

openmp/runtime/src/kmp_atomic.cpp

866

Is the ! correct here?

openmp/runtime/src/z_Windows_NT_util.cpp

81

Does X86 mean 32 bit here?

[mstorsjo]

In D138689#3950527, @DavidSpickett wrote:

(The version in z_Linux_util.cpp only handles up to 15 arguments. And contrary to aarch64, z_Linux_asm.S doesn't have any assembly for 32 bit arm.)

So this means that simply extending it to handle 17 arguments wouldn't help because it would want to use the assembly version of the other functions?

I'm not quite sure I understand the question...

The assembly versions of __kmp_invoke_microtask can handle arbitrary numbers of arguments, since they can do the passing of variable numbers of arguments on the stack correctly. The C version of the function, in the MSVC/arm64 case, also handles it for any arbitrary number of arguments by doing a brittle trick with alloca, here: https://github.com/llvm/llvm-project/blob/main/openmp/runtime/src/z_Windows_NT-586_util.cpp#L170-L182

default: {
  // p_argv[6] and onwards must be passed on the stack since 8 registers are
  // already used.
  size_t len = (argc - 6) * sizeof(void *);
  void *argbuf = alloca(len);
  memcpy(argbuf, &p_argv[6], len);
}
  [[fallthrough]];
case 6:
  (*pkfn)(&gtid, &tid, p_argv[0], p_argv[1], p_argv[2], p_argv[3], p_argv[4],
          p_argv[5]);
  break;
}

This brittle use of alloca and memcpy only works on MSVC - Clang optimizes it out altogether, hence D137827 where we used the assembly version of __kmp_invoke_microtask on Windows/aarch64 with Clang too.

For ARM (on existing handled platforms like Linux), it uses a fallback C version from z_Linux_util.cpp: https://github.com/llvm/llvm-project/blob/main/openmp/runtime/src/z_Linux_util.cpp#L2443-L2532

This is hardcoded to handle only up to 15 arguments (and errors out on any higher number). Extending it to handle up to 17, or any number, like in this patch would be trivial. That would fix the https://github.com/llvm/llvm-project/blob/main/openmp/runtime/test/misc_bugs/many-microtask-args.c testcase for arm linux too. But that doesn't fix the root issue that it only handles up to a specific number of arguments. (The comment in https://github.com/llvm/llvm-project/blob/main/openmp/runtime/src/z_Linux_util.cpp#L2447-L2448 which claims that only one argument is needed is wrong - Clang currently doesn't bundle up arguments in a struct.)

I thought this patch would be the simplest way of getting this new build configuration to work (plus achieving the "all tests passing" status).

The proper solution would be to write an assembly version of __kmp_invoke_microtask for ARM too. I actually already did that and have that available locally, but I thought that might be harder to get reviewed/approved (I'm not sure if the runtime is meant to support older ARM architectures than the armv7 I'm used to writing assembly for, etc), so I went with this patch first. I could also reverse them, first post a patch to switch ARM Linux OpenMP over to an assembly version of the function, then layer this patch on top, to use that assembly function for Windows too, just like for aarch64.

openmp/runtime/src/kmp_atomic.cpp

866

Oh, oops, thanks for catching. Actually, this isn't an !, it's an UTF8 non-breaking space. My keyboard layout produces that for alt+space, which is fairly easy to type accidentally after using alt for the |.

openmp/runtime/src/z_Windows_NT_util.cpp

81

Yes, exactly. Ideally this should probably be specifically "#if BITNESS == 32` or something like that, maybe #ifndef _WIN64 or so. But I'm sticking with the existing code style for now.

[mstorsjo]

Remove an accidental non-ascii character.

[DavidSpickett]

The proper solution would be to write an assembly version of __kmp_invoke_microtask for ARM too.

This looks fine on its own now that I understand the purpose.

(I'm not sure if the runtime is meant to support older ARM architectures than the armv7 I'm used to writing assembly for, etc)

I remember from removing armv2/3 support this file openmp/runtime/src/kmp_platform.h. Though that checks for defines all the way back to armv2 so it's almost no use, I highly doubt anyone builds this for v2. The original Raspberry Pi is Armv6, there is still a Debian build for v4 I think. I suspect we don't know what the minimum should be, but keeping the C version isn't a bad option in that case.

Maybe it makes sense to xfail the number of arguments test on arm Windows as well? So when it comes time to fix both are accounted for (if that time is soon we'd remember, but if it's in a year's time, we won't).

[mstorsjo]

In D138689#3950620, @DavidSpickett wrote:

Maybe it makes sense to xfail the number of arguments test on arm Windows as well? So when it comes time to fix both are accounted for (if that time is soon we'd remember, but if it's in a year's time, we won't).

I guess that sounds reasonable too - I'll try to see if I can do that.

[mstorsjo]

Reduced the C implementation of __kmp_invoke_microtask to only handle 15 arguments, exactly like the same implementation used for ARM on Linux in z_Linux_util.cpp.

This makes the mingw/armv7 build succeed as much as builds on ARM on Linux.

[DavidSpickett]

LGTM, remember to update your commit message to reflect the 15 arguments thing.

openmp/runtime/src/kmp.h

 extern void __kmp_clear_system_time(void);
 extern void __kmp_read_system_time(double *delta);
 
 extern void __kmp_check_stack_overlap(kmp_info_t *thr);
 
 extern void __kmp_expand_host_name(char *buffer, size_t size);
 extern void __kmp_expand_file_name(char *result, size_t rlen, char *pattern);
 
-#if KMP_ARCH_X86 || KMP_ARCH_X86_64 || (KMP_OS_WINDOWS && KMP_ARCH_AARCH64)
+#if KMP_ARCH_X86 || KMP_ARCH_X86_64 || (KMP_OS_WINDOWS && (KMP_ARCH_AARCH64 || KMP_ARCH_ARM))
 extern void
 __kmp_initialize_system_tick(void); /* Initialize timer tick value */
 #endif
 
 extern void
 __kmp_runtime_initialize(void); /* machine specific initialization */
 extern void __kmp_runtime_destroy(void);
 

openmp/runtime/src/kmp_atomic.cpp

while (!KMP_COMPARE_AND_STORE_ACQ##BITS(                                   \
                                                                                \
       *old_value.vvv = *(volatile kmp_int##BITS *)lhs;                         \
       new_value.cmp = (TYPE)(old_value.cmp OP rhs);                            \
     }                                                                          \
   }
 // end of the first part of the workaround for C78287
 #endif // USE_CMPXCHG_FIX
 
-#if KMP_OS_WINDOWS && KMP_ARCH_AARCH64
+#if KMP_OS_WINDOWS && (KMP_ARCH_AARCH64 || KMP_ARCH_ARM)
 // Undo explicit type casts to get MSVC ARM64 to build. Uses
 // OP_CMPXCHG_WORKAROUND definition for OP_CMPXCHG
 #undef OP_CMPXCHG
 #define OP_CMPXCHG(TYPE, BITS, OP)                                             \
   {                                                                            \
     struct _sss {                                                              \
       TYPE cmp;                                                                \
       kmp_int##BITS *vvv;                                                      \
#define OP_CMPXCHG(TYPE, BITS, OP)                                             \
   }
 
 #undef OP_UPDATE_CRITICAL
 #define OP_UPDATE_CRITICAL(TYPE, OP, LCK_ID)                                   \
   __kmp_acquire_atomic_lock(&ATOMIC_LOCK##LCK_ID, gtid);                       \
   (*lhs) = (*lhs)OP rhs;                                                       \
   __kmp_release_atomic_lock(&ATOMIC_LOCK##LCK_ID, gtid);
 
-#endif // KMP_OS_WINDOWS && KMP_ARCH_AARCH64
+#endif // KMP_OS_WINDOWS && (KMP_ARCH_AARCH64 || KMP_ARCH_ARM)
 
 #if KMP_ARCH_X86 || KMP_ARCH_X86_64
 
 // ------------------------------------------------------------------------
 // X86 or X86_64: no alignment problems ====================================
 #define ATOMIC_FIXED_ADD(TYPE_ID, OP_ID, TYPE, BITS, OP, LCK_ID, MASK,         \
                          GOMP_FLAG)                                            \
   ATOMIC_BEGIN(TYPE_ID, OP_ID, TYPE, void)                                     \

openmp/runtime/src/kmp_os.h

 #define KMP_SIZE_T_SPEC KMP_UINT32_SPEC
 #elif KMP_ARCH_X86_64 || KMP_ARCH_PPC64 || KMP_ARCH_AARCH64 ||                 \
     KMP_ARCH_MIPS64 || KMP_ARCH_RISCV64 || KMP_ARCH_LOONGARCH64
 #define KMP_SIZE_T_SPEC KMP_UINT64_SPEC
 #else
 #error "Can't determine size_t printf format specifier."
 #endif
 
-#if KMP_ARCH_X86
+#if KMP_ARCH_X86 || KMP_ARCH_ARM
 #define KMP_SIZE_T_MAX (0xFFFFFFFF)
 #else
 #define KMP_SIZE_T_MAX (0xFFFFFFFFFFFFFFFF)
 #endif
 
 typedef size_t kmp_size_t;
 typedef float kmp_real32;
 typedef double kmp_real64;
enum kmp_mem_fence_type {
   kmp_no_fence, /* No memory fence */
   kmp_acquire_fence, /* Acquire (read) memory fence */
   kmp_release_fence, /* Release (write) memory fence */
   kmp_full_fence /* Full (read+write) memory fence */
 };
 
 // Synchronization primitives
 
-#if KMP_ASM_INTRINS && KMP_OS_WINDOWS && !(KMP_ARCH_AARCH64 && defined(__GNUC__))
+#if KMP_ASM_INTRINS && KMP_OS_WINDOWS && !((KMP_ARCH_AARCH64 || KMP_ARCH_ARM) && defined(__GNUC__))
 
 #if KMP_MSVC_COMPAT && !KMP_COMPILER_CLANG
 #pragma intrinsic(InterlockedExchangeAdd)
 #pragma intrinsic(InterlockedCompareExchange)
 #pragma intrinsic(InterlockedExchange)
 #if !(KMP_COMPILER_ICX && KMP_32_BIT_ARCH)
 #pragma intrinsic(InterlockedExchange64)
 #endif

openmp/runtime/src/kmp_platform.h

 
 #if KMP_OS_WINDOWS
 #if defined(_M_AMD64) || defined(__x86_64)
 #undef KMP_ARCH_X86_64
 #define KMP_ARCH_X86_64 1
 #elif defined(__aarch64__) || defined(_M_ARM64)
 #undef KMP_ARCH_AARCH64
 #define KMP_ARCH_AARCH64 1
+#elif defined(__arm__) || defined(_M_ARM)
+#undef KMP_ARCH_ARMV7
+#define KMP_ARCH_ARMV7 1
 #else
 #undef KMP_ARCH_X86
 #define KMP_ARCH_X86 1
 #endif
 #endif
 
 #if KMP_OS_UNIX
 #if defined __x86_64

openmp/runtime/src/z_Windows_NT-586_util.cpp

 /*
  * z_Windows_NT-586_util.cpp -- platform specific routines.
  */
 
 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "kmp.h"
 
-#if (KMP_ARCH_X86 || KMP_ARCH_X86_64 || KMP_ARCH_AARCH64)
+#if (KMP_ARCH_X86 || KMP_ARCH_X86_64 || KMP_ARCH_AARCH64 || KMP_ARCH_ARM)
 /* Only 32-bit "add-exchange" instruction on IA-32 architecture causes us to
    use compare_and_store for these routines */
 
 kmp_int8 __kmp_test_then_or8(volatile kmp_int8 *p, kmp_int8 d) {
   kmp_int8 old_value, new_value;
 
   old_value = TCR_1(*p);
   new_value = old_value | d;

 #if OMPT_SUPPORT
   *exit_frame_ptr = 0;
 #endif
 
   return 1;
 }
 #endif
 
-#endif /* KMP_ARCH_X86 || KMP_ARCH_X86_64 || KMP_ARCH_AARCH64 */
+#if KMP_ARCH_ARM
+// This matches the generic fallback implementation of __kmp_invoke_microtask
+// from z_Linux_util.cpp, which is used on Linux on ARM.
+int __kmp_invoke_microtask(microtask_t pkfn, int gtid, int tid, int argc,
+                           void *p_argv[]
+#if OMPT_SUPPORT
+                           ,
+                           void **exit_frame_ptr
+#endif
+) {
+#if OMPT_SUPPORT
+  *exit_frame_ptr = OMPT_GET_FRAME_ADDRESS(0);
+#endif
+
+  switch (argc) {
+  default:
+    fprintf(stderr, "Too many args to microtask: %d!\n", argc);
+    fflush(stderr);
+    exit(-1);
+  case 0:
+    (*pkfn)(&gtid, &tid);
+    break;
+  case 1:
+    (*pkfn)(&gtid, &tid, p_argv[0]);
+    break;
+  case 2:
+    (*pkfn)(&gtid, &tid, p_argv[0], p_argv[1]);
+    break;
+  case 3:
+    (*pkfn)(&gtid, &tid, p_argv[0], p_argv[1], p_argv[2]);
+    break;
+  case 4:
+    (*pkfn)(&gtid, &tid, p_argv[0], p_argv[1], p_argv[2], p_argv[3]);
+    break;
+  case 5:
+    (*pkfn)(&gtid, &tid, p_argv[0], p_argv[1], p_argv[2], p_argv[3], p_argv[4]);
+    break;
+  case 6:
+    (*pkfn)(&gtid, &tid, p_argv[0], p_argv[1], p_argv[2], p_argv[3], p_argv[4],
+            p_argv[5]);
+    break;
+  case 7:
+    (*pkfn)(&gtid, &tid, p_argv[0], p_argv[1], p_argv[2], p_argv[3], p_argv[4],
+            p_argv[5], p_argv[6]);
+    break;
+  case 8:
+    (*pkfn)(&gtid, &tid, p_argv[0], p_argv[1], p_argv[2], p_argv[3], p_argv[4],
+            p_argv[5], p_argv[6], p_argv[7]);
+    break;
+  case 9:
+    (*pkfn)(&gtid, &tid, p_argv[0], p_argv[1], p_argv[2], p_argv[3], p_argv[4],
+            p_argv[5], p_argv[6], p_argv[7], p_argv[8]);
+    break;
+  case 10:
+    (*pkfn)(&gtid, &tid, p_argv[0], p_argv[1], p_argv[2], p_argv[3], p_argv[4],
+            p_argv[5], p_argv[6], p_argv[7], p_argv[8], p_argv[9]);
+    break;
+  case 11:
+    (*pkfn)(&gtid, &tid, p_argv[0], p_argv[1], p_argv[2], p_argv[3], p_argv[4],
+            p_argv[5], p_argv[6], p_argv[7], p_argv[8], p_argv[9], p_argv[10]);
+    break;
+  case 12:
+    (*pkfn)(&gtid, &tid, p_argv[0], p_argv[1], p_argv[2], p_argv[3], p_argv[4],
+            p_argv[5], p_argv[6], p_argv[7], p_argv[8], p_argv[9], p_argv[10],
+            p_argv[11]);
+    break;
+  case 13:
+    (*pkfn)(&gtid, &tid, p_argv[0], p_argv[1], p_argv[2], p_argv[3], p_argv[4],
+            p_argv[5], p_argv[6], p_argv[7], p_argv[8], p_argv[9], p_argv[10],
+            p_argv[11], p_argv[12]);
+    break;
+  case 14:
+    (*pkfn)(&gtid, &tid, p_argv[0], p_argv[1], p_argv[2], p_argv[3], p_argv[4],
+            p_argv[5], p_argv[6], p_argv[7], p_argv[8], p_argv[9], p_argv[10],
+            p_argv[11], p_argv[12], p_argv[13]);
+    break;
+  case 15:
+    (*pkfn)(&gtid, &tid, p_argv[0], p_argv[1], p_argv[2], p_argv[3], p_argv[4],
+            p_argv[5], p_argv[6], p_argv[7], p_argv[8], p_argv[9], p_argv[10],
+            p_argv[11], p_argv[12], p_argv[13], p_argv[14]);
+    break;
+  }
+
+#if OMPT_SUPPORT
+  *exit_frame_ptr = 0;
+#endif
+
+  return 1;
+}
+#endif
+
+#endif /* KMP_ARCH_X86 || KMP_ARCH_X86_64 || KMP_ARCH_AARCH64 || KMP_ARCH_ARM */

openmp/runtime/src/z_Windows_NT_util.cpp

struct SYSTEM_THREAD {
   DWORD Priority;
   LONG BasePriority;
   ULONG ContextSwitchCount;
   THREAD_STATE State;
   ULONG WaitReason;
 }; // SYSTEM_THREAD
 
 KMP_BUILD_ASSERT(offsetof(SYSTEM_THREAD, KernelTime) == 0);
-#if KMP_ARCH_X86
+#if KMP_ARCH_X86 || KMP_ARCH_ARM
 KMP_BUILD_ASSERT(offsetof(SYSTEM_THREAD, StartAddress) == 28);
 KMP_BUILD_ASSERT(offsetof(SYSTEM_THREAD, State) == 52);
 #else
 KMP_BUILD_ASSERT(offsetof(SYSTEM_THREAD, StartAddress) == 32);
 KMP_BUILD_ASSERT(offsetof(SYSTEM_THREAD, State) == 68);
 #endif
 
 struct SYSTEM_PROCESS_INFORMATION {
struct SYSTEM_PROCESS_INFORMATION {
   IO_COUNTERS IOCounters;
   SYSTEM_THREAD Threads[1];
 }; // SYSTEM_PROCESS_INFORMATION
 typedef SYSTEM_PROCESS_INFORMATION *PSYSTEM_PROCESS_INFORMATION;
 
 KMP_BUILD_ASSERT(offsetof(SYSTEM_PROCESS_INFORMATION, NextEntryOffset) == 0);
 KMP_BUILD_ASSERT(offsetof(SYSTEM_PROCESS_INFORMATION, CreateTime) == 32);
 KMP_BUILD_ASSERT(offsetof(SYSTEM_PROCESS_INFORMATION, ImageName) == 56);
-#if KMP_ARCH_X86
+#if KMP_ARCH_X86 || KMP_ARCH_ARM
 KMP_BUILD_ASSERT(offsetof(SYSTEM_PROCESS_INFORMATION, ProcessId) == 68);
 KMP_BUILD_ASSERT(offsetof(SYSTEM_PROCESS_INFORMATION, HandleCount) == 76);
 KMP_BUILD_ASSERT(offsetof(SYSTEM_PROCESS_INFORMATION, VMCounters) == 88);
 KMP_BUILD_ASSERT(offsetof(SYSTEM_PROCESS_INFORMATION, IOCounters) == 136);
 KMP_BUILD_ASSERT(offsetof(SYSTEM_PROCESS_INFORMATION, Threads) == 184);
 #else
 KMP_BUILD_ASSERT(offsetof(SYSTEM_PROCESS_INFORMATION, ProcessId) == 80);
 KMP_BUILD_ASSERT(offsetof(SYSTEM_PROCESS_INFORMATION, HandleCount) == 96);