[Openmp-commits] [openmp] [OpenMP] Use simple VLA implementation to replace uses of actual VLA (PR #71412)
Shilei Tian via Openmp-commits
openmp-commits at lists.llvm.org
Mon Nov 6 20:36:01 PST 2023
https://github.com/shiltian updated https://github.com/llvm/llvm-project/pull/71412
>From 85d2013fb5ba045bb3a941a636f47fdc8c607923 Mon Sep 17 00:00:00 2001
From: Shilei Tian <i at tianshilei.me>
Date: Mon, 6 Nov 2023 11:07:57 -0500
Subject: [PATCH 1/4] [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 heap-based VLA that can miminc the behavior of an actual VLA
and prevent the warning.
---
openmp/runtime/src/kmp_gsupport.cpp | 7 +++---
openmp/runtime/src/kmp_runtime.cpp | 3 ++-
openmp/runtime/src/kmp_utils.h | 39 +++++++++++++++++++++++++++++
3 files changed, 45 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..b17bbdb91cedab4
--- /dev/null
+++ b/openmp/runtime/src/kmp_utils.h
@@ -0,0 +1,39 @@
+/*
+ * 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.
+/// Similar to the actual VLA, we don't check boundary (for now), so we will not
+/// store the size. We can always revise it later.
+template <typename T> class SimpleVLA final {
+ T *Data = nullptr;
+
+public:
+ SimpleVLA(std::size_t Size) noexcept : Data(new T[Size]) {}
+
+ ~SimpleVLA() noexcept {
+ if (Data)
+ delete[] Data;
+ }
+
+ const T &operator[](std::size_t Idx) const { return Data[Idx]; }
+ T &operator[](std::size_t Idx) { return Data[Idx]; }
+
+ operator T *() { return Data; }
+ operator const T *() const { return Data; }
+};
+
+#endif
>From a3b5f825dcc1e0240210a5c15edcda68498eee51 Mon Sep 17 00:00:00 2001
From: Shilei Tian <i at tianshilei.me>
Date: Mon, 6 Nov 2023 13:29:22 -0500
Subject: [PATCH 2/4] fix comments
---
openmp/runtime/src/kmp_utils.h | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/openmp/runtime/src/kmp_utils.h b/openmp/runtime/src/kmp_utils.h
index b17bbdb91cedab4..650796c06c2aa1f 100644
--- a/openmp/runtime/src/kmp_utils.h
+++ b/openmp/runtime/src/kmp_utils.h
@@ -22,18 +22,21 @@ template <typename T> class SimpleVLA final {
T *Data = nullptr;
public:
- SimpleVLA(std::size_t Size) noexcept : Data(new T[Size]) {}
+ SimpleVLA() = delete;
+ SimpleVLA(const SimpleVLA &) = delete;
+ SimpleVLA(SimpleVLA &&) = default;
+ SimpleVLA &operator=(const SimpleVLA &) = delete;
+ SimpleVLA &operator=(SimpleVLA &&) = default;
- ~SimpleVLA() noexcept {
- if (Data)
- delete[] Data;
- }
+ explicit SimpleVLA(std::size_t Size) noexcept : Data(new T[Size]) {}
- const T &operator[](std::size_t Idx) const { return Data[Idx]; }
- T &operator[](std::size_t Idx) { return Data[Idx]; }
+ ~SimpleVLA() { delete[] Data; }
- operator T *() { return Data; }
- operator const T *() const { return Data; }
+ const T &operator[](std::size_t Idx) const noexcept { return Data[Idx]; }
+ T &operator[](std::size_t Idx) noexcept { return Data[Idx]; }
+
+ operator T *() noexcept { return Data; }
+ operator const T *() const noexcept { return Data; }
};
#endif
>From 73dc412b48e6210f26024d2ebbae9f3e15b655c6 Mon Sep 17 00:00:00 2001
From: Shilei Tian <i at tianshilei.me>
Date: Mon, 6 Nov 2023 23:04:43 -0500
Subject: [PATCH 3/4] Fix move constructors
---
openmp/runtime/src/kmp_utils.h | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/openmp/runtime/src/kmp_utils.h b/openmp/runtime/src/kmp_utils.h
index 650796c06c2aa1f..9ae327bef718c72 100644
--- a/openmp/runtime/src/kmp_utils.h
+++ b/openmp/runtime/src/kmp_utils.h
@@ -24,14 +24,19 @@ template <typename T> class SimpleVLA final {
public:
SimpleVLA() = delete;
SimpleVLA(const SimpleVLA &) = delete;
- SimpleVLA(SimpleVLA &&) = default;
SimpleVLA &operator=(const SimpleVLA &) = delete;
- SimpleVLA &operator=(SimpleVLA &&) = default;
explicit SimpleVLA(std::size_t Size) noexcept : Data(new T[Size]) {}
~SimpleVLA() { delete[] Data; }
+ SimpleVLA(SimpleVLA &&RHS) : Data(RHS.Data) { RHS.Data = nullptr; }
+
+ SimpleVLA &operator=(SimpleVLA &&RHS) {
+ Data = RHS.Data;
+ RHS.Data = nullptr;
+ }
+
const T &operator[](std::size_t Idx) const noexcept { return Data[Idx]; }
T &operator[](std::size_t Idx) noexcept { return Data[Idx]; }
>From 4826c4fce4d91a26eb5b1583ebc263c545143bca Mon Sep 17 00:00:00 2001
From: Shilei Tian <i at tianshilei.me>
Date: Mon, 6 Nov 2023 23:35:30 -0500
Subject: [PATCH 4/4] Disallow move constructors
---
openmp/runtime/src/kmp_utils.h | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/openmp/runtime/src/kmp_utils.h b/openmp/runtime/src/kmp_utils.h
index 9ae327bef718c72..1097996ee209f6a 100644
--- a/openmp/runtime/src/kmp_utils.h
+++ b/openmp/runtime/src/kmp_utils.h
@@ -24,19 +24,14 @@ template <typename T> class SimpleVLA final {
public:
SimpleVLA() = delete;
SimpleVLA(const SimpleVLA &) = delete;
+ SimpleVLA(SimpleVLA &&) = delete;
SimpleVLA &operator=(const SimpleVLA &) = delete;
+ SimpleVLA &operator=(SimpleVLA &&) = delete;
explicit SimpleVLA(std::size_t Size) noexcept : Data(new T[Size]) {}
~SimpleVLA() { delete[] Data; }
- SimpleVLA(SimpleVLA &&RHS) : Data(RHS.Data) { RHS.Data = nullptr; }
-
- SimpleVLA &operator=(SimpleVLA &&RHS) {
- Data = RHS.Data;
- RHS.Data = nullptr;
- }
-
const T &operator[](std::size_t Idx) const noexcept { return Data[Idx]; }
T &operator[](std::size_t Idx) noexcept { return Data[Idx]; }
More information about the Openmp-commits
mailing list