[clang] [offload-arch] Do not print "-strict" if it is disabled by the environment (PR #224688)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-backend-amdgpu

Author: A. Cauble (accauble)

<details>
<summary>Changes</summary>

## Motivation

On an A0, setting `HSA_DISABLE_GFX12_STRICT=1` in your environment means that rocminfo will print "gfx1250" instead of "gfx1250-strict". This adds a helper to respect the that variable.

## Testing

* This was tested on an A0. When the env var is not set, we see "gfx1250-strict". When the env var is set, we see "gfx1250", matching rocminfo.
* Updated the unit tests to keep track of the environment variable and to check for the expected value depending on it.

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


2 Files Affected:

- (modified) clang/tools/offload-arch/AMDGPUArchByKFD.cpp (+11-2) 
- (modified) clang/unittests/offload-arch/OffloadArchTest.cpp (+60) 


``````````diff
diff --git a/clang/tools/offload-arch/AMDGPUArchByKFD.cpp b/clang/tools/offload-arch/AMDGPUArchByKFD.cpp
index 5720c5a0958b4..3b7a88ab0adcc 100644
--- a/clang/tools/offload-arch/AMDGPUArchByKFD.cpp
+++ b/clang/tools/offload-arch/AMDGPUArchByKFD.cpp
@@ -17,9 +17,9 @@
 #include "llvm/Support/LineIterator.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/Process.h"
 #include "llvm/Support/raw_ostream.h"
 #include <memory>
-#include <tuple>
 
 using namespace llvm;
 
@@ -33,9 +33,18 @@ constexpr static long getMajor(long Ver) { return (Ver / 10000) % 100; }
 constexpr static long getMinor(long Ver) { return (Ver / 100) % 100; }
 constexpr static long getStep(long Ver) { return Ver % 100; }
 
+// HSA_DISABLE_GFX12_STRICT=1 will disable the "-strict" suffix on A0
+static bool isStrictDisabled() {
+  auto DisableEnvVar = sys::Process::GetEnv("HSA_DISABLE_GFX12_STRICT");
+  return (DisableEnvVar.has_value() && DisableEnvVar.value() == "1");
+}
+
 // For A0, print gfx1250-strict to match rocminfo
 static StringRef getRevisionSuffix(long GFXVersion, long ASICRevision) {
-  return (GFXVersion == GFX1250_VERSION && ASICRevision == 0) ? "-strict" : "";
+  return (GFXVersion == GFX1250_VERSION && ASICRevision == 0 &&
+          !isStrictDisabled())
+             ? "-strict"
+             : "";
 }
 
 // Exposed for testing
diff --git a/clang/unittests/offload-arch/OffloadArchTest.cpp b/clang/unittests/offload-arch/OffloadArchTest.cpp
index 8132a6ca10847..f9851cf221e4d 100644
--- a/clang/unittests/offload-arch/OffloadArchTest.cpp
+++ b/clang/unittests/offload-arch/OffloadArchTest.cpp
@@ -11,10 +11,13 @@
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/Process.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Testing/Support/SupportHelpers.h"
 #include "gtest/gtest.h"
 #include <algorithm>
+#include <cstdlib>
+#include <optional>
 #include <string>
 
 // Defined in AMDGPUArchByHIP.cpp (non-static, compiled into this test).
@@ -163,6 +166,39 @@ int printGPUsByKFDCapturingStdout(StringRef NodePath, std::string &Output) {
   Output = testing::internal::GetCapturedStdout();
   return Result;
 }
+
+// RAII helper to set an environment variable for the duration of a test
+// Based on the class of the same name in llvm's Jobserver unit tests
+class ScopedEnvironment {
+  std::string Name;
+  std::optional<std::string> OldValue;
+
+  static void setEnv(const std::string &Name, std::optional<StringRef> Value) {
+#if defined(_WIN32)
+    // On Windows, setting an environment variable to the empty string
+    // unsets it, so getenv() returns NULL
+    _putenv_s(Name.c_str(), Value ? Value->str().c_str() : "");
+#else
+    if (Value)
+      setenv(Name.c_str(), Value->str().c_str(), 1);
+    else
+      unsetenv(Name.c_str());
+#endif
+  }
+
+public:
+  ScopedEnvironment(StringRef Name, std::optional<StringRef> Value)
+      : Name(Name.str()), OldValue(sys::Process::GetEnv(Name)) {
+    setEnv(this->Name, Value);
+  }
+
+  ~ScopedEnvironment() {
+    setEnv(Name, OldValue ? std::optional<StringRef>(*OldValue) : std::nullopt);
+  }
+
+  ScopedEnvironment(const ScopedEnvironment &) = delete;
+  ScopedEnvironment &operator=(const ScopedEnvironment &) = delete;
+};
 } // namespace
 
 // A topology directory that cannot be opened must be reported as a failure, so
@@ -225,6 +261,29 @@ TEST(KFDTopology, MultipleGPUsArePrintedInNodeOrder) {
 // ASIC revision is 0. Also tests to make sure other properties that look like
 // capability (like capability2) are not read instead.
 TEST(KFDTopology, GFX1250A0IsPrintedAsStrict) {
+  ScopedEnvironment Env("HSA_DISABLE_GFX12_STRICT", std::nullopt);
+  unittest::TempDir Dir("kfd-topology", /*Unique=*/true);
+  addGPUNodeWithCapability(Dir.path(), 0, "120500", /*Capability=*/0xF837A280,
+                           /*Capability2=*/0xFFFFFFFF);
+  std::string Output;
+  EXPECT_EQ(printGPUsByKFDCapturingStdout(Dir.path(), Output), 0);
+  EXPECT_EQ(Output, "gfx1250-strict\n");
+}
+
+// HSA_DISABLE_GFX12_STRICT=1 suppresses the suffix on A0.
+TEST(KFDTopology, GFX1250A0StrictSuppressedByEnvVar) {
+  ScopedEnvironment Env("HSA_DISABLE_GFX12_STRICT", "1");
+  unittest::TempDir Dir("kfd-topology", /*Unique=*/true);
+  addGPUNodeWithCapability(Dir.path(), 0, "120500", /*Capability=*/0xF837A280,
+                           /*Capability2=*/0xFFFFFFFF);
+  std::string Output;
+  EXPECT_EQ(printGPUsByKFDCapturingStdout(Dir.path(), Output), 0);
+  EXPECT_EQ(Output, "gfx1250\n");
+}
+
+// Only the exact value "1" disables the suffix.
+TEST(KFDTopology, GFX1250A0StrictIgnoresOtherEnvValues) {
+  ScopedEnvironment Env("HSA_DISABLE_GFX12_STRICT", "0");
   unittest::TempDir Dir("kfd-topology", /*Unique=*/true);
   addGPUNodeWithCapability(Dir.path(), 0, "120500", /*Capability=*/0xF837A280,
                            /*Capability2=*/0xFFFFFFFF);
@@ -235,6 +294,7 @@ TEST(KFDTopology, GFX1250A0IsPrintedAsStrict) {
 
 // Make sure any other version of gfx1250 is printed as gfx1250.
 TEST(KFDTopology, GFX1250NonA0IsPrintedPlain) {
+  ScopedEnvironment Env("HSA_DISABLE_GFX12_STRICT", std::nullopt);
   unittest::TempDir Dir("kfd-topology", /*Unique=*/true);
   addGPUNodeWithCapability(Dir.path(), 0, "120500", /*Capability=*/0xF877A280,
                            /*Capability2=*/0x00000000);

``````````

</details>


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