[Openmp-commits] [openmp] [OpenMP] Use simple VLA implementation to replace uses of actual VLA (PR #71412)

[Shilei Tian via Openmp-commits]

https://github.com/shiltian updated https://github.com/llvm/llvm-project/pull/71412

>From 6e234e17723a3c3134d2bd1516f6d9d7aeeb813f Mon Sep 17 00:00:00 2001
From: Shilei Tian <i at tianshilei.me>
Date: Wed, 8 Nov 2023 17:52:33 -0500
Subject: [PATCH] [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.
---
 openmp/runtime/src/kmp_gsupport.cpp |  7 ++--
 openmp/runtime/src/kmp_runtime.cpp  |  3 +-
 openmp/runtime/src/kmp_utils.h      | 52 +++++++++++++++++++++++++++++
 3 files changed, 58 insertions(+), 4 deletions(-)
 create mode 100644 openmp/runtime/src/kmp_utils.h

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..0c99e340a8d3266
--- /dev/null
+++ b/openmp/runtime/src/kmp_utils.h
@@ -0,0 +1,52 @@
+/*
+ * 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>
+
+/// 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(std::size_t NumOfElements) noexcept {
+    if (NumOfElements > N) {
+      HeapBuffer = new T[NumOfElements];
+      Ptr = HeapBuffer;
+    }
+  }
+
+  ~SimpleVLA() { delete[] 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