[OpenMP][NFC] Separate Envar (environment variable) handling #73994
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
|
@llvm/pr-subscribers-openmp Author: Johannes Doerfert (jdoerfert) ChangesFull diff: https://github.com/llvm/llvm-project/pull/73994.diff 8 Files Affected:
diff --git a/openmp/libomptarget/include/Shared/EnvironmentVar.h b/openmp/libomptarget/include/Shared/EnvironmentVar.h
new file mode 100644
index 000000000000000..4cbdad695a0ee14
--- /dev/null
+++ b/openmp/libomptarget/include/Shared/EnvironmentVar.h
@@ -0,0 +1,194 @@
+//===-- Shared/EnvironmentVar.h - Environment variable handling -*- C++ -*-===//
+//
+// 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 OMPTARGET_SHARED_ENVIRONMENT_VAR_H
+#define OMPTARGET_SHARED_ENVIRONMENT_VAR_H
+
+#include "Debug.h"
+
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+
+#include <sstream>
+#include <string>
+
+/// Utility class for parsing strings to other types.
+struct StringParser {
+ /// Parse a string to another type.
+ template <typename Ty> static bool parse(const char *Value, Ty &Result);
+};
+
+/// Class for reading and checking environment variables. Currently working with
+/// integer, floats, std::string and bool types.
+template <typename Ty> class Envar {
+ Ty Data;
+ bool IsPresent;
+ bool Initialized;
+
+public:
+ /// Auxiliary function to safely create envars. This static function safely
+ /// creates envars using fallible constructors. See the constructors to know
+ /// more details about the creation parameters.
+ template <typename... ArgsTy>
+ static llvm::Expected<Envar> create(ArgsTy &&...Args) {
+ llvm::Error Err = llvm::Error::success();
+ Envar Envar(std::forward<ArgsTy>(Args)..., Err);
+ if (Err)
+ return std::move(Err);
+ return std::move(Envar);
+ }
+
+ /// Create an empty envar. Cannot be consulted. This constructor is merely
+ /// for convenience. This constructor is not fallible.
+ Envar() : Data(Ty()), IsPresent(false), Initialized(false) {}
+
+ /// Create an envar with a name and an optional default. The Envar object will
+ /// take the value read from the environment variable, or the default if it
+ /// was not set or not correct. This constructor is not fallible.
+ Envar(llvm::StringRef Name, Ty Default = Ty())
+ : Data(Default), IsPresent(false), Initialized(true) {
+
+ if (const char *EnvStr = getenv(Name.data())) {
+ // Check whether the envar is defined and valid.
+ IsPresent = StringParser::parse<Ty>(EnvStr, Data);
+
+ if (!IsPresent) {
+ DP("Ignoring invalid value %s for envar %s\n", EnvStr, Name.data());
+ Data = Default;
+ }
+ }
+ }
+
+ Envar<Ty> &operator=(const Ty &V) {
+ Data = V;
+ Initialized = true;
+ return *this;
+ }
+
+ /// Get the definitive value.
+ const Ty &get() const {
+ // Throw a runtime error in case this envar is not initialized.
+ if (!Initialized)
+ FATAL_MESSAGE0(1, "Consulting envar before initialization");
+
+ return Data;
+ }
+
+ /// Get the definitive value.
+ operator Ty() const { return get(); }
+
+ /// Indicate whether the environment variable was defined and valid.
+ bool isPresent() const { return IsPresent; }
+
+private:
+ /// This constructor should never fail but we provide it for convenience. This
+ /// way, the constructor can be used by the Envar::create() static function
+ /// to safely create this kind of envars.
+ Envar(llvm::StringRef Name, Ty Default, llvm::Error &Err)
+ : Envar(Name, Default) {
+ llvm::ErrorAsOutParameter EAO(&Err);
+ Err = llvm::Error::success();
+ }
+
+ /// Create an envar with a name, getter function and a setter function. The
+ /// Envar object will take the value read from the environment variable if
+ /// this value is accepted by the setter function. Otherwise, the getter
+ /// function will be executed to get the default value. The getter should be
+ /// of the form Error GetterFunctionTy(Ty &Value) and the setter should
+ /// be of the form Error SetterFunctionTy(Ty Value). This constructor has a
+ /// private visibility because is a fallible constructor. Please use the
+ /// Envar::create() static function to safely create this object instead.
+ template <typename GetterFunctor, typename SetterFunctor>
+ Envar(llvm::StringRef Name, GetterFunctor Getter, SetterFunctor Setter,
+ llvm::Error &Err)
+ : Data(Ty()), IsPresent(false), Initialized(true) {
+ llvm::ErrorAsOutParameter EAO(&Err);
+ Err = init(Name, Getter, Setter);
+ }
+
+ template <typename GetterFunctor, typename SetterFunctor>
+ llvm::Error init(llvm::StringRef Name, GetterFunctor Getter,
+ SetterFunctor Setter);
+};
+
+/// Define some common envar types.
+using IntEnvar = Envar<int>;
+using Int32Envar = Envar<int32_t>;
+using Int64Envar = Envar<int64_t>;
+using UInt32Envar = Envar<uint32_t>;
+using UInt64Envar = Envar<uint64_t>;
+using StringEnvar = Envar<std::string>;
+using BoolEnvar = Envar<bool>;
+
+template <>
+inline bool StringParser::parse(const char *ValueStr, bool &Result) {
+ std::string Value(ValueStr);
+
+ // Convert the string to lowercase.
+ std::transform(Value.begin(), Value.end(), Value.begin(),
+ [](unsigned char c) { return std::tolower(c); });
+
+ // May be implemented with fancier C++ features, but let's keep it simple.
+ if (Value == "true" || Value == "yes" || Value == "on" || Value == "1")
+ Result = true;
+ else if (Value == "false" || Value == "no" || Value == "off" || Value == "0")
+ Result = false;
+ else
+ return false;
+
+ // Parsed correctly.
+ return true;
+}
+
+template <typename Ty>
+inline bool StringParser::parse(const char *Value, Ty &Result) {
+ assert(Value && "Parsed value cannot be null");
+
+ std::istringstream Stream(Value);
+ Stream >> Result;
+
+ return !Stream.fail();
+}
+
+template <typename Ty>
+template <typename GetterFunctor, typename SetterFunctor>
+inline llvm::Error Envar<Ty>::init(llvm::StringRef Name, GetterFunctor Getter,
+ SetterFunctor Setter) {
+ // Get the default value.
+ Ty Default;
+ if (llvm::Error Err = Getter(Default))
+ return Err;
+
+ if (const char *EnvStr = getenv(Name.data())) {
+ IsPresent = StringParser::parse<Ty>(EnvStr, Data);
+ if (IsPresent) {
+ // Check whether the envar value is actually valid.
+ llvm::Error Err = Setter(Data);
+ if (Err) {
+ // The setter reported an invalid value. Mark the user-defined value as
+ // not present and reset to the getter value (default).
+ IsPresent = false;
+ Data = Default;
+ DP("Setter of envar %s failed, resetting to %s\n", Name.data(),
+ std::to_string(Data).data());
+ consumeError(std::move(Err));
+ }
+ } else {
+ DP("Ignoring invalid value %s for envar %s\n", EnvStr, Name.data());
+ Data = Default;
+ }
+ } else {
+ Data = Default;
+ }
+
+ return llvm::Error::success();
+}
+
+#endif // OMPTARGET_SHARED_ENVIRONMENT_VAR_H
diff --git a/openmp/libomptarget/include/Shared/Utils.h b/openmp/libomptarget/include/Shared/Utils.h
index b6bb97ce59496cc..fce14b54edb9892 100644
--- a/openmp/libomptarget/include/Shared/Utils.h
+++ b/openmp/libomptarget/include/Shared/Utils.h
@@ -15,131 +15,18 @@
#define OMPTARGET_SHARED_UTILS_H
#include "llvm/ADT/StringRef.h"
-#include "llvm/Support/Error.h"
#include "Debug.h"
-#include <algorithm>
#include <atomic>
#include <cassert>
-#include <cstddef>
-#include <cstdint>
-#include <cstdlib>
-#include <functional>
#include <limits>
#include <memory>
-#include <sstream>
-#include <string>
namespace llvm {
namespace omp {
namespace target {
-/// Utility class for parsing strings to other types.
-struct StringParser {
- /// Parse a string to another type.
- template <typename Ty> static bool parse(const char *Value, Ty &Result);
-};
-
-/// Class for reading and checking environment variables. Currently working with
-/// integer, floats, std::string and bool types.
-template <typename Ty> class Envar {
- Ty Data;
- bool IsPresent;
- bool Initialized;
-
-public:
- /// Auxiliary function to safely create envars. This static function safely
- /// creates envars using fallible constructors. See the constructors to know
- /// more details about the creation parameters.
- template <typename... ArgsTy>
- static Expected<Envar> create(ArgsTy &&...Args) {
- Error Err = Error::success();
- Envar Envar(std::forward<ArgsTy>(Args)..., Err);
- if (Err)
- return std::move(Err);
- return std::move(Envar);
- }
-
- /// Create an empty envar. Cannot be consulted. This constructor is merely
- /// for convenience. This constructor is not fallible.
- Envar() : Data(Ty()), IsPresent(false), Initialized(false) {}
-
- /// Create an envar with a name and an optional default. The Envar object will
- /// take the value read from the environment variable, or the default if it
- /// was not set or not correct. This constructor is not fallible.
- Envar(StringRef Name, Ty Default = Ty())
- : Data(Default), IsPresent(false), Initialized(true) {
-
- if (const char *EnvStr = getenv(Name.data())) {
- // Check whether the envar is defined and valid.
- IsPresent = StringParser::parse<Ty>(EnvStr, Data);
-
- if (!IsPresent) {
- DP("Ignoring invalid value %s for envar %s\n", EnvStr, Name.data());
- Data = Default;
- }
- }
- }
-
- Envar<Ty> &operator=(const Ty &V) {
- Data = V;
- Initialized = true;
- return *this;
- }
-
- /// Get the definitive value.
- const Ty &get() const {
- // Throw a runtime error in case this envar is not initialized.
- if (!Initialized)
- FATAL_MESSAGE0(1, "Consulting envar before initialization");
-
- return Data;
- }
-
- /// Get the definitive value.
- operator Ty() const { return get(); }
-
- /// Indicate whether the environment variable was defined and valid.
- bool isPresent() const { return IsPresent; }
-
-private:
- /// This constructor should never fail but we provide it for convenience. This
- /// way, the constructor can be used by the Envar::create() static function
- /// to safely create this kind of envars.
- Envar(StringRef Name, Ty Default, Error &Err) : Envar(Name, Default) {
- ErrorAsOutParameter EAO(&Err);
- Err = Error::success();
- }
-
- /// Create an envar with a name, getter function and a setter function. The
- /// Envar object will take the value read from the environment variable if
- /// this value is accepted by the setter function. Otherwise, the getter
- /// function will be executed to get the default value. The getter should be
- /// of the form Error GetterFunctionTy(Ty &Value) and the setter should
- /// be of the form Error SetterFunctionTy(Ty Value). This constructor has a
- /// private visibility because is a fallible constructor. Please use the
- /// Envar::create() static function to safely create this object instead.
- template <typename GetterFunctor, typename SetterFunctor>
- Envar(StringRef Name, GetterFunctor Getter, SetterFunctor Setter, Error &Err)
- : Data(Ty()), IsPresent(false), Initialized(true) {
- ErrorAsOutParameter EAO(&Err);
- Err = init(Name, Getter, Setter);
- }
-
- template <typename GetterFunctor, typename SetterFunctor>
- Error init(StringRef Name, GetterFunctor Getter, SetterFunctor Setter);
-};
-
-/// Define some common envar types.
-using IntEnvar = Envar<int>;
-using Int32Envar = Envar<int32_t>;
-using Int64Envar = Envar<int64_t>;
-using UInt32Envar = Envar<uint32_t>;
-using UInt64Envar = Envar<uint64_t>;
-using StringEnvar = Envar<std::string>;
-using BoolEnvar = Envar<bool>;
-
/// Utility class for thread-safe reference counting. Any class that needs
/// objects' reference counting can inherit from this entity or have it as a
/// class data member.
@@ -170,70 +57,6 @@ struct RefCountTy {
std::atomic<Ty> Refs;
};
-template <>
-inline bool StringParser::parse(const char *ValueStr, bool &Result) {
- std::string Value(ValueStr);
-
- // Convert the string to lowercase.
- std::transform(Value.begin(), Value.end(), Value.begin(),
- [](unsigned char c) { return std::tolower(c); });
-
- // May be implemented with fancier C++ features, but let's keep it simple.
- if (Value == "true" || Value == "yes" || Value == "on" || Value == "1")
- Result = true;
- else if (Value == "false" || Value == "no" || Value == "off" || Value == "0")
- Result = false;
- else
- return false;
-
- // Parsed correctly.
- return true;
-}
-
-template <typename Ty>
-inline bool StringParser::parse(const char *Value, Ty &Result) {
- assert(Value && "Parsed value cannot be null");
-
- std::istringstream Stream(Value);
- Stream >> Result;
-
- return !Stream.fail();
-}
-
-template <typename Ty>
-template <typename GetterFunctor, typename SetterFunctor>
-inline Error Envar<Ty>::init(StringRef Name, GetterFunctor Getter,
- SetterFunctor Setter) {
- // Get the default value.
- Ty Default;
- if (Error Err = Getter(Default))
- return Err;
-
- if (const char *EnvStr = getenv(Name.data())) {
- IsPresent = StringParser::parse<Ty>(EnvStr, Data);
- if (IsPresent) {
- // Check whether the envar value is actually valid.
- Error Err = Setter(Data);
- if (Err) {
- // The setter reported an invalid value. Mark the user-defined value as
- // not present and reset to the getter value (default).
- IsPresent = false;
- Data = Default;
- DP("Setter of envar %s failed, resetting to %s\n", Name.data(),
- std::to_string(Data).data());
- consumeError(std::move(Err));
- }
- } else {
- DP("Ignoring invalid value %s for envar %s\n", EnvStr, Name.data());
- Data = Default;
- }
- } else {
- Data = Default;
- }
-
- return Error::success();
-}
-
/// Return the difference (in bytes) between \p Begin and \p End.
template <typename Ty = char>
ptrdiff_t getPtrDiff(const void *End, const void *Begin) {
diff --git a/openmp/libomptarget/plugins-nextgen/common/include/JIT.h b/openmp/libomptarget/plugins-nextgen/common/include/JIT.h
index 1a5e56b2ea732a6..7252519a8c2eb3a 100644
--- a/openmp/libomptarget/plugins-nextgen/common/include/JIT.h
+++ b/openmp/libomptarget/plugins-nextgen/common/include/JIT.h
@@ -11,6 +11,7 @@
#ifndef OPENMP_LIBOMPTARGET_PLUGINS_NEXTGEN_COMMON_JIT_H
#define OPENMP_LIBOMPTARGET_PLUGINS_NEXTGEN_COMMON_JIT_H
+#include "Shared/EnvironmentVar.h"
#include "Shared/Utils.h"
#include "llvm/ADT/StringMap.h"
@@ -106,18 +107,16 @@ struct JITEngine {
std::mutex ComputeUnitMapMutex;
/// Control environment variables.
- target::StringEnvar ReplacementObjectFileName =
- target::StringEnvar("LIBOMPTARGET_JIT_REPLACEMENT_OBJECT");
- target::StringEnvar ReplacementModuleFileName =
- target::StringEnvar("LIBOMPTARGET_JIT_REPLACEMENT_MODULE");
- target::StringEnvar PreOptIRModuleFileName =
- target::StringEnvar("LIBOMPTARGET_JIT_PRE_OPT_IR_MODULE");
- target::StringEnvar PostOptIRModuleFileName =
- target::StringEnvar("LIBOMPTARGET_JIT_POST_OPT_IR_MODULE");
- target::UInt32Envar JITOptLevel =
- target::UInt32Envar("LIBOMPTARGET_JIT_OPT_LEVEL", 3);
- target::BoolEnvar JITSkipOpt =
- target::BoolEnvar("LIBOMPTARGET_JIT_SKIP_OPT", false);
+ StringEnvar ReplacementObjectFileName =
+ StringEnvar("LIBOMPTARGET_JIT_REPLACEMENT_OBJECT");
+ StringEnvar ReplacementModuleFileName =
+ StringEnvar("LIBOMPTARGET_JIT_REPLACEMENT_MODULE");
+ StringEnvar PreOptIRModuleFileName =
+ StringEnvar("LIBOMPTARGET_JIT_PRE_OPT_IR_MODULE");
+ StringEnvar PostOptIRModuleFileName =
+ StringEnvar("LIBOMPTARGET_JIT_POST_OPT_IR_MODULE");
+ UInt32Envar JITOptLevel = UInt32Envar("LIBOMPTARGET_JIT_OPT_LEVEL", 3);
+ BoolEnvar JITSkipOpt = BoolEnvar("LIBOMPTARGET_JIT_SKIP_OPT", false);
};
} // namespace target
diff --git a/openmp/libomptarget/plugins-nextgen/common/include/MemoryManager.h b/openmp/libomptarget/plugins-nextgen/common/include/MemoryManager.h
index 95491b4be6fa6f0..fe1989930b76ef7 100644
--- a/openmp/libomptarget/plugins-nextgen/common/include/MemoryManager.h
+++ b/openmp/libomptarget/plugins-nextgen/common/include/MemoryManager.h
@@ -324,7 +324,7 @@ 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() {
- static llvm::omp::target::UInt32Envar MemoryManagerThreshold(
+ static UInt32Envar MemoryManagerThreshold(
"LIBOMPTARGET_MEMORY_MANAGER_THRESHOLD", 0);
size_t Threshold = MemoryManagerThreshold.get();
diff --git a/openmp/libomptarget/plugins-nextgen/common/include/PluginInterface.h b/openmp/libomptarget/plugins-nextgen/common/include/PluginInterface.h
index 6abd1b6829ab554..dd601e6b4231aae 100644
--- a/openmp/libomptarget/plugins-nextgen/common/include/PluginInterface.h
+++ b/openmp/libomptarget/plugins-nextgen/common/include/PluginInterface.h
@@ -21,6 +21,7 @@
#include "Shared/Debug.h"
#include "Shared/Environment.h"
+#include "Shared/EnvironmentVar.h"
#include "Shared/Utils.h"
#include "GlobalHandler.h"
diff --git a/openmp/libomptarget/src/device.cpp b/openmp/libomptarget/src/device.cpp
index 2431d7c073352ab..da7d33b3f40c629 100644
--- a/openmp/libomptarget/src/device.cpp
+++ b/openmp/libomptarget/src/device.cpp
@@ -18,7 +18,7 @@
#include "private.h"
#include "rtl.h"
-#include "Shared/Utils.h"
+#include "Shared/EnvironmentVar.h"
#include <cassert>
#include <climits>
@@ -535,11 +535,10 @@ void DeviceTy::init() {
return;
// Enables recording kernels if set.
- llvm::omp::target::BoolEnvar OMPX_RecordKernel("LIBOMPTARGET_RECORD", false);
+ BoolEnvar OMPX_RecordKernel("LIBOMPTARGET_RECORD", false);
if (OMPX_RecordKernel) {
// Enables saving the device memory kernel output post execution if set.
- llvm::omp::target::BoolEnvar OMPX_ReplaySaveOutput(
- "LIBOMPTARGET_RR_SAVE_OUTPUT", false);
+ BoolEnvar OMPX_ReplaySaveOutput("LIBOMPTARGET_RR_SAVE_OUTPUT", false);
uint64_t ReqPtrArgOffset;
RTL->initialize_record_replay(RTLDeviceID, 0, nullptr, true,
diff --git a/openmp/libomptarget/src/interface.cpp b/openmp/libomptarget/src/interface.cpp
index 97cada94527f0ff..a2f713459e1d0c9 100644
--- a/openmp/libomptarget/src/interface.cpp
+++ b/openmp/libomptarget/src/interface.cpp
@@ -19,8 +19,8 @@
#include "private.h"
#include "rtl.h"
+#include "Shared/EnvironmentVar.h"
#include "Shared/Profile.h"
-#include "Shared/Utils.h"
#include "Utils/ExponentialBackoff.h"
diff --git a/openmp/libomptarget/src/rtl.cpp b/openmp/libomptarget/src/rtl.cpp
index e80d6e09c4840bc..09084be12a76e2a 100644
--- a/openmp/libomptarget/src/rtl.cpp
+++ b/openmp/libomptarget/src/rtl.cpp
@@ -14,11 +14,11 @@
#include "OpenMP/OMPT/Callback.h"
#include "PluginManager.h"
-#include "Shared/Debug.h"
#include "device.h"
#include "private.h"
#include "rtl.h"
+#include "Shared/Debug.h"
#include "Shared/Profile.h"
#include "Shared/Utils.h"
|
jhuber6
approved these changes
Nov 30, 2023
DominikAdamski
added a commit
to DominikAdamski/llvm-project-upstream
that referenced
this pull request
Dec 1, 2023
Libomptarget cannot be build because of the recent refactoring introduced in patch 148dec9 : [OpenMP][NFC] Separate Envar (environment variable) handling (llvm#73994) This patch moved handling of environment variables from libomptarget library. That's why we don't need usage of "llvm::omp::target" namespace if we handle environment variables.
DominikAdamski
added a commit
that referenced
this pull request
Dec 1, 2023
Libomptarget cannot be build because of the recent refactoring introduced in patch 148dec9 : [OpenMP][NFC] Separate Envar (environment variable) handling (#73994) That patch moved handling of environment variables from libomptarget library. That's why we don't need usage of "llvm::omp::target" namespace if we handle environment variables.
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.
No description provided.