-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[OpenMP][NFC] Merge MemoryManager into PluginInterface #73678
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Similar to llvm#73677, there is no benefit from keeping MemoryManager seperate; it's tied into the current design. Except the move I also replaced the getenv call with our Env handling.
@llvm/pr-subscribers-backend-amdgpu @llvm/pr-subscribers-openmp Author: Johannes Doerfert (jdoerfert) ChangesSimilar to #73677, there is no benefit from keeping MemoryManager seperate; it's tied into the current design. Except the move I also replaced the getenv call with our Env handling. Full diff: https://github.com/llvm/llvm-project/pull/73678.diff 8 Files Affected:
diff --git a/openmp/libomptarget/plugins-nextgen/CMakeLists.txt b/openmp/libomptarget/plugins-nextgen/CMakeLists.txt
index 966f51621c322d5..a27dd0a4ca9186c 100644
--- a/openmp/libomptarget/plugins-nextgen/CMakeLists.txt
+++ b/openmp/libomptarget/plugins-nextgen/CMakeLists.txt
@@ -47,7 +47,6 @@ if(CMAKE_SYSTEM_PROCESSOR MATCHES "${tmachine}$")
LINK_LIBS
PRIVATE
- MemoryManager
PluginInterface
${LIBOMPTARGET_DEP_LIBFFI_LIBRARIES}
${OPENMP_PTHREAD_LIB}
diff --git a/openmp/libomptarget/plugins-nextgen/amdgpu/CMakeLists.txt b/openmp/libomptarget/plugins-nextgen/amdgpu/CMakeLists.txt
index 94a0084db6077ad..af967bc386f67dc 100644
--- a/openmp/libomptarget/plugins-nextgen/amdgpu/CMakeLists.txt
+++ b/openmp/libomptarget/plugins-nextgen/amdgpu/CMakeLists.txt
@@ -80,7 +80,6 @@ add_llvm_library(omptarget.rtl.amdgpu SHARED
LINK_LIBS
PRIVATE
- MemoryManager
PluginInterface
${LIBOMPTARGET_DEP_LIBRARIES}
${OPENMP_PTHREAD_LIB}
diff --git a/openmp/libomptarget/plugins-nextgen/common/CMakeLists.txt b/openmp/libomptarget/plugins-nextgen/common/CMakeLists.txt
index 3d1f2634e6bc496..f0645d0d1753817 100644
--- a/openmp/libomptarget/plugins-nextgen/common/CMakeLists.txt
+++ b/openmp/libomptarget/plugins-nextgen/common/CMakeLists.txt
@@ -12,4 +12,3 @@
add_subdirectory(OMPT)
add_subdirectory(PluginInterface)
-add_subdirectory(MemoryManager)
diff --git a/openmp/libomptarget/plugins-nextgen/common/MemoryManager/CMakeLists.txt b/openmp/libomptarget/plugins-nextgen/common/MemoryManager/CMakeLists.txt
deleted file mode 100644
index 7f2e7c7c8d39fe0..000000000000000
--- a/openmp/libomptarget/plugins-nextgen/common/MemoryManager/CMakeLists.txt
+++ /dev/null
@@ -1,11 +0,0 @@
-##===----------------------------------------------------------------------===##
-#
-# 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
-#
-##===----------------------------------------------------------------------===##
-
-add_library(MemoryManager INTERFACE)
-
-target_include_directories(MemoryManager INTERFACE ${CMAKE_CURRENT_SOURCE_DIR})
diff --git a/openmp/libomptarget/plugins-nextgen/common/OMPT/CMakeLists.txt b/openmp/libomptarget/plugins-nextgen/common/OMPT/CMakeLists.txt
index db79c6d3e550ee5..be4c743665b3e97 100644
--- a/openmp/libomptarget/plugins-nextgen/common/OMPT/CMakeLists.txt
+++ b/openmp/libomptarget/plugins-nextgen/common/OMPT/CMakeLists.txt
@@ -52,7 +52,6 @@ endif()
target_link_libraries(OMPT
PUBLIC
${llvm_libs}
- MemoryManager
)
# Define the TARGET_NAME and DEBUG_PREFIX.
diff --git a/openmp/libomptarget/plugins-nextgen/common/PluginInterface/CMakeLists.txt b/openmp/libomptarget/plugins-nextgen/common/PluginInterface/CMakeLists.txt
index 4ff4dee3c0875f7..84a7dc8ea72ddc3 100644
--- a/openmp/libomptarget/plugins-nextgen/common/PluginInterface/CMakeLists.txt
+++ b/openmp/libomptarget/plugins-nextgen/common/PluginInterface/CMakeLists.txt
@@ -63,7 +63,6 @@ endif()
target_link_libraries(PluginInterface
PUBLIC
${llvm_libs}
- MemoryManager
)
# Include the RPC server from the `libc` project if availible.
diff --git a/openmp/libomptarget/plugins-nextgen/common/MemoryManager/MemoryManager.h b/openmp/libomptarget/plugins-nextgen/common/PluginInterface/MemoryManager.h
similarity index 95%
rename from openmp/libomptarget/plugins-nextgen/common/MemoryManager/MemoryManager.h
rename to openmp/libomptarget/plugins-nextgen/common/PluginInterface/MemoryManager.h
index 37ef80a1af3ae22..2d23d959f6c0ba0 100644
--- a/openmp/libomptarget/plugins-nextgen/common/MemoryManager/MemoryManager.h
+++ b/openmp/libomptarget/plugins-nextgen/common/PluginInterface/MemoryManager.h
@@ -10,8 +10,8 @@
//
//===----------------------------------------------------------------------===//
-#ifndef LLVM_OPENMP_LIBOMPTARGET_PLUGINS_COMMON_MEMORYMANAGER_MEMORYMANAGER_H
-#define LLVM_OPENMP_LIBOMPTARGET_PLUGINS_COMMON_MEMORYMANAGER_MEMORYMANAGER_H
+#ifndef LLVM_OPENMP_LIBOMPTARGET_PLUGINS_COMMON_MEMORYMANAGER_H
+#define LLVM_OPENMP_LIBOMPTARGET_PLUGINS_COMMON_MEMORYMANAGER_H
#include <cassert>
#include <functional>
@@ -22,6 +22,8 @@
#include <vector>
#include "Debug.h"
+#include "Utilities.h"
+#include "omptarget.h"
/// Base class of per-device allocator.
class DeviceAllocatorTy {
@@ -322,16 +324,15 @@ class MemoryManagerTy {
/// manager explicitly by setting the var to 0. If user doesn't specify
/// anything, returns <0, true>.
static std::pair<size_t, bool> getSizeThresholdFromEnv() {
- size_t Threshold = 0;
-
- if (const char *Env =
- std::getenv("LIBOMPTARGET_MEMORY_MANAGER_THRESHOLD")) {
- Threshold = std::stoul(Env);
- if (Threshold == 0) {
- DP("Disabled memory manager as user set "
- "LIBOMPTARGET_MEMORY_MANAGER_THRESHOLD=0.\n");
- return std::make_pair(0, false);
- }
+ static llvm::omp::target::UInt32Envar MemoryManagerThreshold(
+ "LIBOMPTARGET_MEMORY_MANAGER_THRESHOLD", 0);
+
+ size_t Threshold = MemoryManagerThreshold.get();
+
+ if (MemoryManagerThreshold.isPresent() && Threshold == 0) {
+ DP("Disabled memory manager as user set "
+ "LIBOMPTARGET_MEMORY_MANAGER_THRESHOLD=0.\n");
+ return std::make_pair(0, false);
}
return std::make_pair(Threshold, true);
@@ -343,4 +344,4 @@ class MemoryManagerTy {
constexpr const size_t MemoryManagerTy::BucketSize[];
constexpr const int MemoryManagerTy::NumBuckets;
-#endif // LLVM_OPENMP_LIBOMPTARGET_PLUGINS_COMMON_MEMORYMANAGER_MEMORYMANAGER_H
+#endif // LLVM_OPENMP_LIBOMPTARGET_PLUGINS_COMMON_MEMORYMANAGER_H
diff --git a/openmp/libomptarget/plugins-nextgen/cuda/CMakeLists.txt b/openmp/libomptarget/plugins-nextgen/cuda/CMakeLists.txt
index d0e6b26888d5532..3e1d11ac365df64 100644
--- a/openmp/libomptarget/plugins-nextgen/cuda/CMakeLists.txt
+++ b/openmp/libomptarget/plugins-nextgen/cuda/CMakeLists.txt
@@ -34,7 +34,6 @@ add_llvm_library(omptarget.rtl.cuda SHARED
Object
LINK_LIBS PRIVATE
- MemoryManager
PluginInterface
${OPENMP_PTHREAD_LIB}
|
jhuber6
approved these changes
Nov 28, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Similar to #73677, there is no benefit from keeping MemoryManager seperate; it's tied into the current design. Except the move I also replaced the getenv call with our Env handling.