[Openmp-commits] [openmp] a4d1d5f - [OpenMP] Use simple VLA implementation to replace uses of actual VLA

[Shilei Tian via Openmp-commits]

Author: Shilei Tian
Date: 2023-12-15T15:12:33-05:00
New Revision: a4d1d5f5b54b2f93d7290588734f59ff24fc515c

URL: https://github.com/llvm/llvm-project/commit/a4d1d5f5b54b2f93d7290588734f59ff24fc515c
DIFF: https://github.com/llvm/llvm-project/commit/a4d1d5f5b54b2f93d7290588734f59ff24fc515c.diff

LOG: [OpenMP] Use simple VLA implementation to replace uses of actual VLA

Use of VLA can cause compile warning that was introduced in D156565. This patch
implements a simple stack/heap-based VLA that can miminc the behavior of an
actual VLA and prevent the warning. By default the stack accomodates the
elements. If the number of emelements is greater than N, which by default is 8,
a heap buffer will be allocated and used to acccomodate the elements.

Added: 
    openmp/runtime/src/kmp_utils.h

Modified: 
    openmp/runtime/src/kmp_gsupport.cpp
    openmp/runtime/src/kmp_runtime.cpp

Removed: 
    


################################################################################
diff  --git a/openmp/runtime/src/kmp_gsupport.cpp b/openmp/runtime/src/kmp_gsupport.cpp
index dc843f78dc6975..78af395335498b 100644
--- a/openmp/runtime/src/kmp_gsupport.cpp
+++ b/openmp/runtime/src/kmp_gsupport.cpp
@@ -12,6 +12,7 @@
 
 #include "kmp.h"
 #include "kmp_atomic.h"
+#include "kmp_utils.h"
 
 #if OMPT_SUPPORT
 #include "ompt-specific.h"
@@ -1280,7 +1281,7 @@ void KMP_EXPAND_NAME(KMP_API_NAME_GOMP_TASK)(void (*func)(void *), void *data,
       KMP_ASSERT(depend);
       kmp_gomp_depends_info_t gomp_depends(depend);
       kmp_int32 ndeps = gomp_depends.get_num_deps();
-      kmp_depend_info_t dep_list[ndeps];
+      SimpleVLA<kmp_depend_info_t> dep_list(ndeps);
       for (kmp_int32 i = 0; i < ndeps; i++)
         dep_list[i] = gomp_depends.get_kmp_depend(i);
       kmp_int32 ndeps_cnv;
@@ -1309,7 +1310,7 @@ void KMP_EXPAND_NAME(KMP_API_NAME_GOMP_TASK)(void (*func)(void *), void *data,
       KMP_ASSERT(depend);
       kmp_gomp_depends_info_t gomp_depends(depend);
       kmp_int32 ndeps = gomp_depends.get_num_deps();
-      kmp_depend_info_t dep_list[ndeps];
+      SimpleVLA<kmp_depend_info_t> dep_list(ndeps);
       for (kmp_int32 i = 0; i < ndeps; i++)
         dep_list[i] = gomp_depends.get_kmp_depend(i);
       __kmpc_omp_wait_deps(&loc, gtid, ndeps, dep_list, 0, NULL);
@@ -1993,7 +1994,7 @@ void KMP_EXPAND_NAME(KMP_API_NAME_GOMP_TASKWAIT_DEPEND)(void **depend) {
   KA_TRACE(20, ("GOMP_taskwait_depend: T#%d\n", gtid));
   kmp_gomp_depends_info_t gomp_depends(depend);
   kmp_int32 ndeps = gomp_depends.get_num_deps();
-  kmp_depend_info_t dep_list[ndeps];
+  SimpleVLA<kmp_depend_info_t> dep_list(ndeps);
   for (kmp_int32 i = 0; i < ndeps; i++)
     dep_list[i] = gomp_depends.get_kmp_depend(i);
 #if OMPT_SUPPORT

diff  --git a/openmp/runtime/src/kmp_runtime.cpp b/openmp/runtime/src/kmp_runtime.cpp
index 7d2e7a99ade431..4e1074a893a281 100644
--- a/openmp/runtime/src/kmp_runtime.cpp
+++ b/openmp/runtime/src/kmp_runtime.cpp
@@ -24,6 +24,7 @@
 #include "kmp_wait_release.h"
 #include "kmp_wrapper_getpid.h"
 #include "kmp_dispatch.h"
+#include "kmp_utils.h"
 #if KMP_USE_HIER_SCHED
 #include "kmp_dispatch_hier.h"
 #endif
@@ -1653,7 +1654,7 @@ __kmp_serial_fork_call(ident_t *loc, int gtid, enum fork_context_e call_context,
 /* josh todo: hypothetical question: what do we do for OS X*? */
 #if KMP_OS_LINUX &&                                                            \
     (KMP_ARCH_X86 || KMP_ARCH_X86_64 || KMP_ARCH_ARM || KMP_ARCH_AARCH64)
-  void *args[argc];
+  SimpleVLA<void *> args(argc);
 #else
   void **args = (void **)KMP_ALLOCA(argc * sizeof(void *));
 #endif /* KMP_OS_LINUX && ( KMP_ARCH_X86 || KMP_ARCH_X86_64 || KMP_ARCH_ARM || \

diff  --git a/openmp/runtime/src/kmp_utils.h b/openmp/runtime/src/kmp_utils.h
new file mode 100644
index 00000000000000..a557f929e6e723
--- /dev/null
+++ b/openmp/runtime/src/kmp_utils.h
@@ -0,0 +1,55 @@
+/*
+ * kmp_utils.h -- Utilities that used internally
+ */
+
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+#ifndef __KMP_UTILS_H__
+#define __KMP_UTILS_H__
+
+#include <cstddef>
+
+#include "kmp.h"
+
+/// A simple pure header implementation of VLA that aims to replace uses of
+/// actual VLA, which can cause compile warning. This class by default creates a
+/// stack buffer that can accomodate \p N elements. If the number of elements is
+/// greater than \p N, then a heap buffer will be allocated and used to
+/// accomodate the elements. Similar to the actual VLA, we don't check boundary
+/// (for now), so we will not store the number of elements. We can always revise
+/// it later.
+template <typename T, unsigned N = 8> class SimpleVLA final {
+  T StackBuffer[N];
+  T *HeapBuffer = nullptr;
+  T *Ptr = StackBuffer;
+
+public:
+  SimpleVLA() = delete;
+  SimpleVLA(const SimpleVLA &) = delete;
+  SimpleVLA(SimpleVLA &&) = delete;
+  SimpleVLA &operator=(const SimpleVLA &) = delete;
+  SimpleVLA &operator=(SimpleVLA &&) = delete;
+
+  explicit SimpleVLA(unsigned NumOfElements) noexcept {
+    if (NumOfElements > N) {
+      HeapBuffer =
+          reinterpret_cast<T *>(__kmp_allocate(NumOfElements * sizeof(T)));
+      Ptr = HeapBuffer;
+    }
+  }
+
+  ~SimpleVLA() {
+    if (HeapBuffer)
+      __kmp_free(HeapBuffer);
+  }
+
+  operator T *() noexcept { return Ptr; }
+  operator const T *() const noexcept { return Ptr; }
+};
+
+#endif