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 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
