[Openmp-commits] [PATCH] D137828: RFC: [openmp] Don't assume a specific layout for alloca() in Windows arm64 __kmp_invoke_microtask

[Martin Storsjö via Phabricator via Openmp-commits]

mstorsjo created this revision.
mstorsjo added reviewers: AndreyChurbanov, JonChesterfield, natgla, pulidocr, jdoerfert.
Herald added subscribers: guansong, kristof.beyls, yaxunl.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a subscriber: sstefan1.
Herald added a project: OpenMP.

The alloca + memcpy could be optimized out entirely by Clang,
since it wasn't referenced by anything outside of the function,
and had no visible effect.

And even if it wouldn't be optimized out, there's strictly no
guarantee that the buffer allocated by alloca is at the exact
stack frame boundary when calling the next function; the compiler
could easily have added extra padding around the allocation at the
bottom.

Change to using the same full switch as the fallback in z_Linux_util.cpp,
which supports up to 15 arguments. This fixes calls to microtasks with
more than 6 parameters in optimized builds with Clang. However, for
the build configurations where the alloca wasn't broken already, this
change does break the misc_bugs/many-microtask-args.c test - which
requires passing 17 arguments to the microtask.

This tries to fix the same issue as D137827 <https://reviews.llvm.org/D137827> does, but fixing it in
pure C code. However this fix only works for a set, fixed maximum
number of arguments.

The same implementation is being used as fallback implementation in
z_Linux_util.cpp for architectures without an assembly implementation

- where it also fails the misc_bugs/many-microtask-args.c test.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137828

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


Index: openmp/runtime/src/z_Windows_NT-586_util.cpp
===================================================================
--- openmp/runtime/src/z_Windows_NT-586_util.cpp
+++ openmp/runtime/src/z_Windows_NT-586_util.cpp
@@ -149,6 +149,10 @@
 #endif
 
   switch (argc) {
+  default:
+    fprintf(stderr, "Too many args to microtask: %d!\n", argc);
+    fflush(stderr);
+    exit(-1);
   case 0:
     (*pkfn)(&gtid, &tid);
     break;
@@ -167,18 +171,50 @@
   case 5:
     (*pkfn)(&gtid, &tid, p_argv[0], p_argv[1], p_argv[2], p_argv[3], p_argv[4]);
     break;
-  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;
+  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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D137828.474723.patch
Type: text/x-patch
Size: 2702 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/openmp-commits/attachments/20221111/5ab94a4e/attachment.bin>