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

[Shilei Tian via Openmp-commits]

Author: Shilei Tian
Date: 2023-11-28T19:04:30-05:00
New Revision: 97e16da450e94c92456fa5a74768ec1b22fe6b63

URL: https://github.com/llvm/llvm-project/commit/97e16da450e94c92456fa5a74768ec1b22fe6b63
DIFF: https://github.com/llvm/llvm-project/commit/97e16da450e94c92456fa5a74768ec1b22fe6b63.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 d77d4809a7e95c2..f38a0ddfc62a1fb 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 25136691bc72de9..4ac694ace874220 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
@@ -1652,7 +1653,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 000000000000000..009334792c45218
--- /dev/null
+++ b/openmp/runtime/src/kmp_utils.h
@@ -0,0 +1,58 @@
+/*
+ * 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);
+  }
+
+  const T &operator[](std::size_t Idx) const noexcept { return Ptr[Idx]; }
+  T &operator[](std::size_t Idx) noexcept { return Ptr[Idx]; }
+
+  operator T *() noexcept { return Ptr; }
+  operator const T *() const noexcept { return Ptr; }
+};
+
+#endif