https://github.com/accauble updated https://github.com/llvm/llvm-project/pull/224688
>From 0d1cefd571912eb7e8e6c1fd13a76157393dc3ac Mon Sep 17 00:00:00 2001 From: Allyson Cauble-Chantrenne <[email protected]> Date: Fri, 18 Sep 2026 11:11:06 -0400 Subject: [PATCH 1/4] [offload-arch] Remove stray include --- clang/tools/offload-arch/AMDGPUArchByKFD.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/clang/tools/offload-arch/AMDGPUArchByKFD.cpp b/clang/tools/offload-arch/AMDGPUArchByKFD.cpp index 5720c5a0958b4..315c87b75eb23 100644 --- a/clang/tools/offload-arch/AMDGPUArchByKFD.cpp +++ b/clang/tools/offload-arch/AMDGPUArchByKFD.cpp @@ -19,7 +19,6 @@ #include "llvm/Support/Path.h" #include "llvm/Support/raw_ostream.h" #include <memory> -#include <tuple> using namespace llvm; >From 323e85e9bf9d957cdc20764220ada0fda866305f Mon Sep 17 00:00:00 2001 From: Allyson Cauble-Chantrenne <[email protected]> Date: Fri, 18 Sep 2026 12:39:51 -0400 Subject: [PATCH 2/4] [offload-arch] Disable "-strict" printing if env var says so --- clang/tools/offload-arch/AMDGPUArchByKFD.cpp | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/clang/tools/offload-arch/AMDGPUArchByKFD.cpp b/clang/tools/offload-arch/AMDGPUArchByKFD.cpp index 315c87b75eb23..3b7a88ab0adcc 100644 --- a/clang/tools/offload-arch/AMDGPUArchByKFD.cpp +++ b/clang/tools/offload-arch/AMDGPUArchByKFD.cpp @@ -17,6 +17,7 @@ #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> @@ -32,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 >From ef75dc37c2ae73815935f724a9394f7bed28f581 Mon Sep 17 00:00:00 2001 From: Allyson Cauble-Chantrenne <[email protected]> Date: Mon, 21 Sep 2026 09:58:19 -0400 Subject: [PATCH 3/4] [offload-arch] Updated unit tests to handle new env var --- .../offload-arch/OffloadArchTest.cpp | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) 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); >From 9c2d28359fea9232019cafa2d0f5857b00652e5a Mon Sep 17 00:00:00 2001 From: Allyson Cauble-Chantrenne <[email protected]> Date: Tue, 22 Sep 2026 08:34:34 -0400 Subject: [PATCH 4/4] [offload-arch] Updated DISABLE_STRICT to reflect current default * rocminfo currently requires HSA_DISABLE_GFX12_STRICT=0 in order to report an A0 as gfx1250-strict * My understanding is that this is a temporary change while SW catches up. Added comments regarding that --- clang/tools/offload-arch/AMDGPUArchByKFD.cpp | 11 ++++++----- clang/unittests/offload-arch/OffloadArchTest.cpp | 11 ++++++----- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/clang/tools/offload-arch/AMDGPUArchByKFD.cpp b/clang/tools/offload-arch/AMDGPUArchByKFD.cpp index 3b7a88ab0adcc..1dc41906f40ac 100644 --- a/clang/tools/offload-arch/AMDGPUArchByKFD.cpp +++ b/clang/tools/offload-arch/AMDGPUArchByKFD.cpp @@ -33,16 +33,17 @@ 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"); +// A temporary patch in ROCr has ISA defaulting to gfx1250 on A0 and enabling +// the "-strict" suffix with HSA_DISABLE_GFX12_STRICT=0 +static bool isStrictEnabled() { + auto EnableEnvVar = sys::Process::GetEnv("HSA_DISABLE_GFX12_STRICT"); + return (EnableEnvVar.has_value() && EnableEnvVar.value() == "0"); } // For A0, print gfx1250-strict to match rocminfo static StringRef getRevisionSuffix(long GFXVersion, long ASICRevision) { return (GFXVersion == GFX1250_VERSION && ASICRevision == 0 && - !isStrictDisabled()) + isStrictEnabled()) ? "-strict" : ""; } diff --git a/clang/unittests/offload-arch/OffloadArchTest.cpp b/clang/unittests/offload-arch/OffloadArchTest.cpp index f9851cf221e4d..c380b8c7f1287 100644 --- a/clang/unittests/offload-arch/OffloadArchTest.cpp +++ b/clang/unittests/offload-arch/OffloadArchTest.cpp @@ -261,7 +261,8 @@ 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); + // A temporary patch in ROCr requires this env var to be 0 + ScopedEnvironment Env("HSA_DISABLE_GFX12_STRICT", "0"); unittest::TempDir Dir("kfd-topology", /*Unique=*/true); addGPUNodeWithCapability(Dir.path(), 0, "120500", /*Capability=*/0xF837A280, /*Capability2=*/0xFFFFFFFF); @@ -281,20 +282,20 @@ TEST(KFDTopology, GFX1250A0StrictSuppressedByEnvVar) { EXPECT_EQ(Output, "gfx1250\n"); } -// Only the exact value "1" disables the suffix. +// Temporary test: Only the exact value "0" enables the suffix. TEST(KFDTopology, GFX1250A0StrictIgnoresOtherEnvValues) { - ScopedEnvironment Env("HSA_DISABLE_GFX12_STRICT", "0"); + 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"); + EXPECT_EQ(Output, "gfx1250\n"); } // Make sure any other version of gfx1250 is printed as gfx1250. TEST(KFDTopology, GFX1250NonA0IsPrintedPlain) { - ScopedEnvironment Env("HSA_DISABLE_GFX12_STRICT", std::nullopt); + ScopedEnvironment Env("HSA_DISABLE_GFX12_STRICT", "0"); unittest::TempDir Dir("kfd-topology", /*Unique=*/true); addGPUNodeWithCapability(Dir.path(), 0, "120500", /*Capability=*/0xF877A280, /*Capability2=*/0x00000000); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
