[Openmp-commits] [openmp] [OpenMP][NFC] Separate Envar (environment variable) handling (PR #73994)

via Openmp-commits openmp-commits at lists.llvm.org
Thu Nov 30 14:19:03 PST 2023


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-openmp

Author: Johannes Doerfert (jdoerfert)

<details>
<summary>Changes</summary>



---
Full diff: https://github.com/llvm/llvm-project/pull/73994.diff


8 Files Affected:

- (added) openmp/libomptarget/include/Shared/EnvironmentVar.h (+194) 
- (modified) openmp/libomptarget/include/Shared/Utils.h (-177) 
- (modified) openmp/libomptarget/plugins-nextgen/common/include/JIT.h (+11-12) 
- (modified) openmp/libomptarget/plugins-nextgen/common/include/MemoryManager.h (+1-1) 
- (modified) openmp/libomptarget/plugins-nextgen/common/include/PluginInterface.h (+1) 
- (modified) openmp/libomptarget/src/device.cpp (+3-4) 
- (modified) openmp/libomptarget/src/interface.cpp (+1-1) 
- (modified) openmp/libomptarget/src/rtl.cpp (+1-1) 


``````````diff
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"
 

``````````

</details>


https://github.com/llvm/llvm-project/pull/73994


More information about the Openmp-commits mailing list