From 0cc84793e5188d872fbd2b84611466b4acab9367 Mon Sep 17 00:00:00 2001 From: Damyan Pepper Date: Tue, 11 Aug 2026 21:13:13 -0700 Subject: [PATCH 1/3] Stop requiring HlslDataDir for execution tests ExecHLSLTests could only find its data files through the HlslDataDir parameter or an absolute path baked in at build time, pointing into the builder's source tree. Anything running without that tree had to pass HlslDataDir, and omitting it gave a bare 0x80070002. It now looks beside the test binary too, so a copy deployed alongside its data needs nothing else. An explicit HlslDataDir still takes precedence, and build trees still resolve to the source tree, so XML edits apply without a rebuild. Failures now list the paths tried. Drop the now-redundant setting from hcttest.cmd and taef_exec/lit.cfg. Fixes #7961 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e211a1ad-3bcd-4fbf-a1ce-d8d177660bff --- tools/clang/test/taef_exec/lit.cfg | 5 +- .../unittests/HLSLExec/HlslExecTestUtils.cpp | 116 +++++++++++++++++- utils/hct/hcttest.cmd | 2 +- 3 files changed, 116 insertions(+), 7 deletions(-) diff --git a/tools/clang/test/taef_exec/lit.cfg b/tools/clang/test/taef_exec/lit.cfg index af52cd8867..f3c2d4dce2 100644 --- a/tools/clang/test/taef_exec/lit.cfg +++ b/tools/clang/test/taef_exec/lit.cfg @@ -47,12 +47,9 @@ Execution tests require DXIL.dll to sign shaders, or the experimental shader mod if config.unsupported == False: test_dll = os.path.join(bin_dir, 'ExecHLSLTests.dll') - hlsl_data_dir = os.path.join(config.llvm_src_root, 'tools', 'clang', 'unittests', 'HLSLExec') - test_dir = os.path.join(config.llvm_obj_root, config.llvm_build_mode, 'test') - param_hlsl_data_dir = str.format('HlslDataDir={}', hlsl_data_dir) - extra_params = ['/p:', 'ExperimentalShaders=*', '/p:', param_hlsl_data_dir] + extra_params = ['/p:', 'ExperimentalShaders=*'] verbose = bool(getattr(config, 'verbose', False)) if verbose != True: diff --git a/tools/clang/unittests/HLSLExec/HlslExecTestUtils.cpp b/tools/clang/unittests/HLSLExec/HlslExecTestUtils.cpp index 91b813ed1e..eaa7d2f039 100644 --- a/tools/clang/unittests/HLSLExec/HlslExecTestUtils.cpp +++ b/tools/clang/unittests/HLSLExec/HlslExecTestUtils.cpp @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -425,6 +426,118 @@ static bool createDevice( return true; } +// Directory of this module, resolved by address rather than by name so it +// works whatever the binary is called. +static const std::wstring &getContainingModuleDirectory() { + static const std::wstring Dir = [] { + HMODULE Module = nullptr; + if (!GetModuleHandleExW( + GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS | + GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT, + reinterpret_cast(&getContainingModuleDirectory), &Module)) + return std::wstring(); + + // GetModuleFileNameW truncates rather than failing, so grow until it fits. + std::wstring Path(MAX_PATH, L'\0'); + for (;;) { + const DWORD Len = + GetModuleFileNameW(Module, &Path[0], static_cast(Path.size())); + if (Len == 0) + return std::wstring(); + if (Len < Path.size()) { + Path.resize(Len); + break; + } + Path.resize(Path.size() * 2); + } + return std::filesystem::path(Path).parent_path().wstring(); + }(); + return Dir; +} + +// Every call site loads from the same directory, so log the first resolution +// only. Atomic because tests may run in parallel. +static void logResolvedDataDirOnce(const wchar_t *Origin, + const std::filesystem::path &Path) { + static std::atomic Logged{false}; + if (Logged.exchange(true)) + return; + LogCommentFmt(L"Loading execution test data from %s: %s", Origin, + Path.parent_path().c_str()); +} + +// Resolves a data file shipped with the execution tests, in this order: +// +// 1. the HlslDataDir runtime parameter, if supplied. Authoritative: we fail +// rather than fall through, so a typo is reported instead of hidden. +// 2. the directory containing this module, so a binary deployed with its +// data files finds them. +// 3. the source directory recorded by CMake, so a build tree picks up edits +// to the data files without rebuilding. +static std::wstring resolveHlslDataFile(LPCWSTR RelativePath) { + WEX::Common::String ParamValue; + std::wstring OverrideDir; + if (SUCCEEDED(WEX::TestExecution::RuntimeParameters::TryGetValue( + HLSLDATAFILEPARAM, ParamValue))) + OverrideDir = reinterpret_cast(ParamValue.GetBuffer()); + + // Treat an empty value as absent and fall through to the other locations. + if (!OverrideDir.empty()) { + std::filesystem::path Candidate = + std::filesystem::path(OverrideDir) / RelativePath; + if (!std::filesystem::exists(Candidate)) + LOG_ERROR_FMT_THROW( + L"Unable to find %s in the directory given by the %s parameter.\n" + L" Tried: %s\n" + L"%s is authoritative, so no other location was searched. Omit it " + L"to search the test binary's directory and the source directory.", + RelativePath, HLSLDATAFILEPARAM, Candidate.c_str(), + HLSLDATAFILEPARAM); + logResolvedDataDirOnce( + FormatToWString(L"the %s parameter", HLSLDATAFILEPARAM).c_str(), + Candidate); + return Candidate.wstring(); + } + + const std::wstring ModuleDir = getContainingModuleDirectory(); + std::filesystem::path ModuleCandidate; + if (!ModuleDir.empty()) { + ModuleCandidate = std::filesystem::path(ModuleDir) / RelativePath; + if (std::filesystem::exists(ModuleCandidate)) { + logResolvedDataDirOnce(L"the directory containing the test binary", + ModuleCandidate); + return ModuleCandidate.wstring(); + } + } + + // Empty in _HLK_CONF builds; probing it would just look in the current + // directory. + const std::wstring SourceDir = DEFAULT_EXEC_TEST_DIR; + std::filesystem::path SourceCandidate; + if (!SourceDir.empty()) { + SourceCandidate = std::filesystem::path(SourceDir) / RelativePath; + if (std::filesystem::exists(SourceCandidate)) { + logResolvedDataDirOnce(L"the build-configured source directory", + SourceCandidate); + return SourceCandidate.wstring(); + } + } + + LOG_ERROR_FMT_THROW( + L"Unable to find the test data file %s in any known location.\n" + L" Next to the test binary: %s\n" + L" Build-configured source path: %s\n" + L"Deploy %s next to the test binary, or pass " + L"/p:\"%s=\" to specify its location.", + RelativePath, + ModuleCandidate.empty() ? L"" + : ModuleCandidate.c_str(), + SourceCandidate.empty() ? L"" + : SourceCandidate.c_str(), + RelativePath, HLSLDATAFILEPARAM, RelativePath); + return std::wstring(); +} + void readHlslDataIntoNewStream(LPCWSTR RelativePath, IStream **Stream, dxc::SpecificDllLoader &Support) { VERIFY_SUCCEEDED( @@ -432,8 +545,7 @@ void readHlslDataIntoNewStream(LPCWSTR RelativePath, IStream **Stream, CComPtr Library; CComPtr Blob; CComPtr StreamCom; - std::wstring Path = GetPathToHlslDataFile(RelativePath, HLSLDATAFILEPARAM, - DEFAULT_EXEC_TEST_DIR); + std::wstring Path = resolveHlslDataFile(RelativePath); VERIFY_SUCCEEDED(Support.CreateInstance(CLSID_DxcLibrary, &Library)); VERIFY_SUCCEEDED(Library->CreateBlobFromFile(Path.c_str(), nullptr, &Blob)); VERIFY_SUCCEEDED(Library->CreateStreamFromBlobReadOnly(Blob, &StreamCom)); diff --git a/utils/hct/hcttest.cmd b/utils/hct/hcttest.cmd index 3e81d207e5..eebc531c91 100644 --- a/utils/hct/hcttest.cmd +++ b/utils/hct/hcttest.cmd @@ -454,7 +454,7 @@ if "%TEST_EXEC%"=="1" ( call :copyagility ) -set EXEC_COMMON_ARGS=/p:"HlslDataDir=%HLSL_SRC_DIR%\tools\clang\unittests\HLSLExec" /p:"ExperimentalShaders=*" %TEST_ADAPTER% %USE_AGILITY_SDK% +set EXEC_COMMON_ARGS=/p:"ExperimentalShaders=*" %TEST_ADAPTER% %USE_AGILITY_SDK% if "%TEST_EXEC%"=="1" ( echo Sniffing for D3D12 configuration ... call :runte ExecHLSLTests.dll /select:"@Name='ExecutionTest::BasicTriangleTest' AND @Architecture='%TEST_ARCH%'" %EXEC_COMMON_ARGS% From 677a02a1b4544f989668828edbc1741bc2ff5c44 Mon Sep 17 00:00:00 2001 From: Damyan Pepper Date: Wed, 12 Aug 2026 11:12:02 -0700 Subject: [PATCH 2/3] Use __ImageBase --- .../unittests/HLSLExec/HlslExecTestUtils.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tools/clang/unittests/HLSLExec/HlslExecTestUtils.cpp b/tools/clang/unittests/HLSLExec/HlslExecTestUtils.cpp index eaa7d2f039..06cfa93b83 100644 --- a/tools/clang/unittests/HLSLExec/HlslExecTestUtils.cpp +++ b/tools/clang/unittests/HLSLExec/HlslExecTestUtils.cpp @@ -426,16 +426,16 @@ static bool createDevice( return true; } -// Directory of this module, resolved by address rather than by name so it -// works whatever the binary is called. +// Linker-provided symbol at the base of the module containing this code, which +// is also its HMODULE. Identifies the module without depending on the name of +// the binary these sources are built into. +// See https://devblogs.microsoft.com/oldnewthing/20041025-00/?p=37483. +extern "C" IMAGE_DOS_HEADER __ImageBase; + +// Directory of the module containing this code. static const std::wstring &getContainingModuleDirectory() { static const std::wstring Dir = [] { - HMODULE Module = nullptr; - if (!GetModuleHandleExW( - GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS | - GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT, - reinterpret_cast(&getContainingModuleDirectory), &Module)) - return std::wstring(); + HMODULE Module = reinterpret_cast(&__ImageBase); // GetModuleFileNameW truncates rather than failing, so grow until it fits. std::wstring Path(MAX_PATH, L'\0'); From 21420a4e0a3ca2a0d1538a55cc3ff3542a6c7d56 Mon Sep 17 00:00:00 2001 From: Damyan Pepper Date: Wed, 12 Aug 2026 11:16:51 -0700 Subject: [PATCH 3/3] Use call_once --- .../clang/unittests/HLSLExec/HlslExecTestUtils.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tools/clang/unittests/HLSLExec/HlslExecTestUtils.cpp b/tools/clang/unittests/HLSLExec/HlslExecTestUtils.cpp index 06cfa93b83..3d787dc5f9 100644 --- a/tools/clang/unittests/HLSLExec/HlslExecTestUtils.cpp +++ b/tools/clang/unittests/HLSLExec/HlslExecTestUtils.cpp @@ -7,11 +7,11 @@ #include #include -#include #include #include #include #include +#include #include // D3D12_FEATURE_D3D12_OPTIONS_PREVIEW and its data struct are not yet in @@ -456,14 +456,14 @@ static const std::wstring &getContainingModuleDirectory() { } // Every call site loads from the same directory, so log the first resolution -// only. Atomic because tests may run in parallel. +// only. Tests may run in parallel, so this has to be thread-safe. static void logResolvedDataDirOnce(const wchar_t *Origin, const std::filesystem::path &Path) { - static std::atomic Logged{false}; - if (Logged.exchange(true)) - return; - LogCommentFmt(L"Loading execution test data from %s: %s", Origin, - Path.parent_path().c_str()); + static std::once_flag Logged; + std::call_once(Logged, [&] { + LogCommentFmt(L"Loading execution test data from %s: %s", Origin, + Path.parent_path().c_str()); + }); } // Resolves a data file shipped with the execution tests, in this order: