From 9aaaa225d99409f9b572ef8a427cda147b09052b Mon Sep 17 00:00:00 2001 From: Damyan Pepper Date: Fri, 21 Aug 2026 20:58:44 -0700 Subject: [PATCH] Fix concurrent dxilconv pass initialization Register all passes used by the DXBC converter during DLL startup. Add fresh-process tests for the pass closure and concurrent conversion. Fixes #8819 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1147c945-f9cc-4aa3-8f58-e7238e0270ec --- docs/ReleaseNotes.md | 2 + .../include/DxilConvPasses/InitializePasses.h | 22 +++ .../lib/DxilConvPasses/DxilCleanup.cpp | 2 + .../lib/DxilConvPasses/InitializePasses.cpp | 6 + projects/dxilconv/tools/dxilconv/dxilconv.cpp | 8 +- projects/dxilconv/unittests/CMakeLists.txt | 29 ++- .../DxilConvPassRegistryTestHelper.cpp | 179 ++++++++++++++++++ projects/dxilconv/unittests/DxilConvTests.cpp | 65 +++++++ tools/opt/opt.cpp | 6 +- 9 files changed, 314 insertions(+), 5 deletions(-) create mode 100644 projects/dxilconv/include/DxilConvPasses/InitializePasses.h create mode 100644 projects/dxilconv/unittests/DxilConvPassRegistryTestHelper.cpp diff --git a/docs/ReleaseNotes.md b/docs/ReleaseNotes.md index 19c0d75174..22269971c2 100644 --- a/docs/ReleaseNotes.md +++ b/docs/ReleaseNotes.md @@ -24,6 +24,8 @@ line upon naming the release. Refer to previous for appropriate section names. #### Bug Fixes +- Fixed a race that could crash concurrent first-time DXBC-to-DXIL conversions + [#8819](https://github.com/microsoft/DirectXShaderCompiler/issues/8819). - Fixed derivative operations being moved into divergent control flow, which could produce incorrect results [#8001](https://github.com/microsoft/DirectXShaderCompiler/issues/8001). diff --git a/projects/dxilconv/include/DxilConvPasses/InitializePasses.h b/projects/dxilconv/include/DxilConvPasses/InitializePasses.h new file mode 100644 index 0000000000..a216839799 --- /dev/null +++ b/projects/dxilconv/include/DxilConvPasses/InitializePasses.h @@ -0,0 +1,22 @@ +/////////////////////////////////////////////////////////////////////////////// +// // +// InitializePasses.h // +// Copyright (C) Microsoft Corporation. All rights reserved. // +// This file is distributed under the University of Illinois Open Source // +// License. See LICENSE.TXT for details. // +// // +/////////////////////////////////////////////////////////////////////////////// + +#pragma once + +#include "dxc/Support/Global.h" + +namespace llvm { +class PassRegistry; +} + +void __cdecl initializeDxilConvPasses(llvm::PassRegistry &Registry); + +namespace hlsl { +HRESULT SetupRegistryPassForDxilConvPasses(); +} diff --git a/projects/dxilconv/lib/DxilConvPasses/DxilCleanup.cpp b/projects/dxilconv/lib/DxilConvPasses/DxilCleanup.cpp index 1091d0d4d0..5bce6b9a99 100644 --- a/projects/dxilconv/lib/DxilConvPasses/DxilCleanup.cpp +++ b/projects/dxilconv/lib/DxilConvPasses/DxilCleanup.cpp @@ -1452,6 +1452,8 @@ char &llvm::DxilCleanupID = DxilCleanup::ID; INITIALIZE_PASS_BEGIN(DxilCleanup, "dxil-cleanup", "Optimize DXIL after conversion from DXBC", true, false) +INITIALIZE_PASS_DEPENDENCY(DCE) +INITIALIZE_PASS_DEPENDENCY(PromotePass) INITIALIZE_PASS_END(DxilCleanup, "dxil-cleanup", "Optimize DXIL after conversion from DXBC", true, false) diff --git a/projects/dxilconv/lib/DxilConvPasses/InitializePasses.cpp b/projects/dxilconv/lib/DxilConvPasses/InitializePasses.cpp index 4810971354..0f677540f9 100644 --- a/projects/dxilconv/lib/DxilConvPasses/InitializePasses.cpp +++ b/projects/dxilconv/lib/DxilConvPasses/InitializePasses.cpp @@ -10,6 +10,7 @@ // // /////////////////////////////////////////////////////////////////////////////// +#include "DxilConvPasses/InitializePasses.h" #include "DxilConvPasses/DxilCleanup.h" #include "DxilConvPasses/NormalizeDxil.h" #include "DxilConvPasses/ScopeNestInfo.h" @@ -17,12 +18,17 @@ #include "dxc/Support/Global.h" #include "dxc/Support/WinIncludes.h" +#include "llvm/Analysis/ReducibilityAnalysis.h" +#include "llvm/InitializePasses.h" #include "llvm/PassRegistry.h" using namespace llvm; // Place to put our private pass initialization for opt.exe. void __cdecl initializeDxilConvPasses(PassRegistry &Registry) { + initializeDCEPass(Registry); + initializePromotePassPass(Registry); + initializeReducibilityAnalysisPass(Registry); initializeScopeNestedCFGPass(Registry); initializeScopeNestInfoWrapperPassPass(Registry); initializeNormalizeDxilPassPass(Registry); diff --git a/projects/dxilconv/tools/dxilconv/dxilconv.cpp b/projects/dxilconv/tools/dxilconv/dxilconv.cpp index 73d7f3df0e..61a0a06ddb 100644 --- a/projects/dxilconv/tools/dxilconv/dxilconv.cpp +++ b/projects/dxilconv/tools/dxilconv/dxilconv.cpp @@ -26,6 +26,7 @@ #include "dxcetw.h" #include "DxbcConverter.h" +#include "DxilConvPasses/InitializePasses.h" // Defined in DxbcConverter.lib // (projects/dxilconv/lib/DxbcConverter/DxbcConverter.cpp) @@ -87,7 +88,7 @@ DXC_API_IMPORT HRESULT __stdcall DxcCreateInstance2(IMalloc *pMalloc, // __declspec(nothrow) static HRESULT InitMaybeFail() throw() { HRESULT hr; - bool memSetup = false; + bool fsSetup = false, memSetup = false; IFC(DxcInitThreadMalloc()); DxcSetThreadMallocToDefault(); memSetup = true; @@ -95,8 +96,13 @@ static HRESULT InitMaybeFail() throw() { hr = E_FAIL; goto Cleanup; } + fsSetup = true; + IFC(hlsl::SetupRegistryPassForDxilConvPasses()); Cleanup: if (FAILED(hr)) { + if (fsSetup) { + ::llvm::sys::fs::CleanupPerThreadFileSystem(); + } if (memSetup) { DxcClearThreadMalloc(); DxcCleanupThreadMalloc(); diff --git a/projects/dxilconv/unittests/CMakeLists.txt b/projects/dxilconv/unittests/CMakeLists.txt index 6949493265..ce4e652a3e 100644 --- a/projects/dxilconv/unittests/CMakeLists.txt +++ b/projects/dxilconv/unittests/CMakeLists.txt @@ -2,8 +2,31 @@ # This file is distributed under the University of Illinois Open Source License. See LICENSE.TXT for details. find_package(TAEF REQUIRED) +find_package(D3D12 REQUIRED) #find_package(DiaSDK REQUIRED) # Used for constants and declarations. +set(LLVM_OPTIONAL_SOURCES + DxilConvPassRegistryTestHelper.cpp + DxilConvTests.cpp +) + +add_dxilconv_project_executable(dxilconv-pass-registry-test-helper + DxilConvPassRegistryTestHelper.cpp +) + +target_link_libraries(dxilconv-pass-registry-test-helper PRIVATE + DxilConvPasses + LLVMAnalysis + LLVMCore + LLVMScalarOpts + LLVMSupport + ${D3D12_LIBRARIES} +) + +target_include_directories(dxilconv-pass-registry-test-helper PRIVATE + ${D3D12_INCLUDE_DIRS} +) + add_dxilconv_project_test_library(dxilconv-tests SHARED DxilConvTests.cpp ) @@ -26,11 +49,15 @@ target_include_directories(dxilconv-tests PRIVATE # dxilconv-tests calls out to several external tools. Those need to be listed # here to ensure they build with dxilconv for our test target depencencies to be # correct. -add_dependencies(dxilconv-tests dxilconv HLSLTestLib dxbc2dxil dxa opt) +add_dependencies(dxilconv-tests dxilconv dxilconv-pass-registry-test-helper + HLSLTestLib dxbc2dxil dxa opt) install(TARGETS dxilconv-tests RUNTIME DESTINATION bin) +install(TARGETS dxilconv-pass-registry-test-helper + RUNTIME DESTINATION bin) + # Add a .user file with settings for te.exe. file(TO_NATIVE_PATH "${CMAKE_CURRENT_SOURCE_DIR}" DOS_STYLE_SOURCE_DIR) file(TO_NATIVE_PATH "${TAEF_BIN_DIR}" DOS_TAEF_BIN_DIR) diff --git a/projects/dxilconv/unittests/DxilConvPassRegistryTestHelper.cpp b/projects/dxilconv/unittests/DxilConvPassRegistryTestHelper.cpp new file mode 100644 index 0000000000..482d3a3f31 --- /dev/null +++ b/projects/dxilconv/unittests/DxilConvPassRegistryTestHelper.cpp @@ -0,0 +1,179 @@ +/////////////////////////////////////////////////////////////////////////////// +// // +// DxilConvPassRegistryTestHelper.cpp // +// Copyright (C) Microsoft Corporation. All rights reserved. // +// This file is distributed under the University of Illinois Open Source // +// License. See LICENSE.TXT for details. // +// // +/////////////////////////////////////////////////////////////////////////////// + +#include "dxc/Support/WinIncludes.h" + +#include "DxbcConverter.h" +#include "DxilConvPasses/InitializePasses.h" +#include "llvm/PassRegistry.h" + +#include +#include + +#include +#include +#include +#include +#include + +static int VerifyPassRegistration() { + HRESULT Result = hlsl::SetupRegistryPassForDxilConvPasses(); + if (FAILED(Result)) + return 1; + + llvm::PassRegistry *Registry = llvm::PassRegistry::getPassRegistry(); + const char *RequiredPasses[] = { + "dce", + "mem2reg", + "assumption-cache-tracker", + "red", + "loops", + "domtree", + "dxil-cleanup", + "normalizedxil", + "scopenested", + "scopenestinfo", + }; + for (const char *PassName : RequiredPasses) { + if (!Registry->getPassInfo(llvm::StringRef(PassName))) { + std::fprintf(stderr, "Pass was not registered: %s\n", PassName); + return 1; + } + } + return 0; +} + +static std::wstring GetDxilConvPath() { + wchar_t Path[MAX_PATH]; + DWORD Length = GetModuleFileNameW(nullptr, Path, _countof(Path)); + if (Length == 0 || Length == _countof(Path)) + return {}; + + std::wstring Result(Path, Length); + size_t Separator = Result.find_last_of(L"\\/"); + if (Separator == std::wstring::npos) + return {}; + Result.resize(Separator + 1); + Result += L"dxilconv.dll"; + return Result; +} + +static bool CompileShaders(std::vector> &Shaders) { + for (size_t Index = 0; Index < Shaders.size(); ++Index) { + std::string Source = + "float4 main(float4 position : SV_Position) : SV_Target {" + " return float4(position.x + " + + std::to_string(Index) + ".0f, position.y, 0.0f, 1.0f); }"; + CComPtr Errors; + HRESULT Result = + D3DCompile(Source.data(), Source.size(), "concurrent-init.hlsl", nullptr, + nullptr, "main", "ps_5_0", D3DCOMPILE_OPTIMIZATION_LEVEL3, 0, + &Shaders[Index], &Errors); + if (FAILED(Result)) { + if (Errors) + std::fwrite(Errors->GetBufferPointer(), 1, Errors->GetBufferSize(), + stderr); + return false; + } + } + return true; +} + +static int RunConcurrentConversions() { + constexpr unsigned WorkerCount = 256; + std::vector> Shaders(WorkerCount); + if (!CompileShaders(Shaders)) + return 1; + + std::wstring DxilConvPath = GetDxilConvPath(); + if (DxilConvPath.empty()) + return 1; + + HMODULE DxilConv = LoadLibraryW(DxilConvPath.c_str()); + if (!DxilConv) + return 1; + + auto CreateInstance = reinterpret_cast( + GetProcAddress(DxilConv, "DxcCreateInstance")); + if (!CreateInstance) { + FreeLibrary(DxilConv); + return 1; + } + + HANDLE ReadyEvent = CreateEventW(nullptr, TRUE, FALSE, nullptr); + HANDLE StartEvent = CreateEventW(nullptr, TRUE, FALSE, nullptr); + if (!ReadyEvent || !StartEvent) { + if (ReadyEvent) + CloseHandle(ReadyEvent); + if (StartEvent) + CloseHandle(StartEvent); + FreeLibrary(DxilConv); + return 1; + } + + std::atomic ReadyCount{0}; + std::vector Results(WorkerCount, E_PENDING); + std::vector Workers; + Workers.reserve(WorkerCount); + + for (unsigned Index = 0; Index < WorkerCount; ++Index) { + Workers.emplace_back([&, Index]() { + IDxbcConverter *RawConverter = nullptr; + HRESULT Result = CreateInstance( + CLSID_DxbcConverter, __uuidof(IDxbcConverter), + reinterpret_cast(&RawConverter)); + CComPtr Converter; + Converter.Attach(RawConverter); + + if (ReadyCount.fetch_add(1, std::memory_order_release) + 1 == WorkerCount) + SetEvent(ReadyEvent); + WaitForSingleObject(StartEvent, INFINITE); + + if (SUCCEEDED(Result)) { + void *Dxil = nullptr; + UINT32 DxilSize = 0; + LPWSTR Diagnostics = nullptr; + Result = Converter->Convert( + Shaders[Index]->GetBufferPointer(), + static_cast(Shaders[Index]->GetBufferSize()), nullptr, &Dxil, + &DxilSize, &Diagnostics); + CoTaskMemFree(Dxil); + CoTaskMemFree(Diagnostics); + } + Results[Index] = Result; + }); + } + + DWORD ReadyResult = WaitForSingleObject(ReadyEvent, 60000); + SetEvent(StartEvent); + for (std::thread &Worker : Workers) + Worker.join(); + + CloseHandle(StartEvent); + CloseHandle(ReadyEvent); + FreeLibrary(DxilConv); + + if (ReadyResult != WAIT_OBJECT_0) + return 1; + for (HRESULT Result : Results) { + if (FAILED(Result)) + return 1; + } + return 0; +} + +int __cdecl wmain(int ArgCount, wchar_t **Arguments) { + if (ArgCount != 2) + return 1; + if (wcscmp(Arguments[1], L"--verify-pass-registration") == 0) + return VerifyPassRegistration(); + if (wcscmp(Arguments[1], L"--concurrent-conversion") == 0) + return RunConcurrentConversions(); + return 1; +} diff --git a/projects/dxilconv/unittests/DxilConvTests.cpp b/projects/dxilconv/unittests/DxilConvTests.cpp index 6fb14337f2..17cb020d0a 100644 --- a/projects/dxilconv/unittests/DxilConvTests.cpp +++ b/projects/dxilconv/unittests/DxilConvTests.cpp @@ -168,6 +168,71 @@ class DxilConvTest { } }; +class DxilConvPassRegistryTest { +public: + BEGIN_TEST_CLASS(DxilConvPassRegistryTest) + TEST_CLASS_PROPERTY(L"Parallel", L"false") + TEST_METHOD_PROPERTY(L"Priority", L"0") + END_TEST_CLASS() + + TEST_METHOD(PassDependenciesRegistered); + TEST_METHOD(ConcurrentInitialization); + +private: + static DWORD RunHelper(LPCWSTR Arguments) { + HMODULE TestModule = GetModuleHandleW(L"dxilconv-tests.dll"); + IFTBOOL(TestModule != nullptr, HRESULT_FROM_WIN32(GetLastError())); + + wchar_t ModulePath[MAX_PATH]; + DWORD Length = + GetModuleFileNameW(TestModule, ModulePath, _countof(ModulePath)); + IFTBOOL(Length != 0 && Length != _countof(ModulePath), + HRESULT_FROM_WIN32(GetLastError())); + + std::wstring HelperPath(ModulePath, Length); + size_t Separator = HelperPath.find_last_of(L"\\/"); + IFTBOOL(Separator != std::wstring::npos, E_FAIL); + HelperPath.resize(Separator + 1); + HelperPath += L"dxilconv-pass-registry-test-helper.exe"; + + std::wstring CommandLine = + L"\"" + HelperPath + L"\" " + std::wstring(Arguments); + std::vector MutableCommandLine(CommandLine.begin(), + CommandLine.end()); + MutableCommandLine.push_back(L'\0'); + + STARTUPINFOW StartupInfo = {}; + StartupInfo.cb = sizeof(StartupInfo); + PROCESS_INFORMATION ProcessInfo = {}; + IFTBOOL(CreateProcessW(HelperPath.c_str(), MutableCommandLine.data(), + nullptr, nullptr, FALSE, 0, nullptr, nullptr, + &StartupInfo, &ProcessInfo), + HRESULT_FROM_WIN32(GetLastError())); + + CloseHandle(ProcessInfo.hThread); + DWORD WaitResult = WaitForSingleObject(ProcessInfo.hProcess, 120000); + if (WaitResult != WAIT_OBJECT_0) { + TerminateProcess(ProcessInfo.hProcess, 1); + CloseHandle(ProcessInfo.hProcess); + IFT(E_FAIL); + } + + DWORD ExitCode = 1; + BOOL GotExitCode = GetExitCodeProcess(ProcessInfo.hProcess, &ExitCode); + CloseHandle(ProcessInfo.hProcess); + IFTBOOL(GotExitCode, HRESULT_FROM_WIN32(GetLastError())); + return ExitCode; + } +}; + +TEST_F(DxilConvPassRegistryTest, PassDependenciesRegistered) { + VERIFY_ARE_EQUAL(0u, RunHelper(L"--verify-pass-registration")); +} + +TEST_F(DxilConvPassRegistryTest, ConcurrentInitialization) { + VERIFY_ARE_EQUAL(0u, RunHelper(L"--concurrent-conversion")); +} + bool DxilConvTest::InitSupport() { if (!m_dllSupport.IsEnabled()) { VERIFY_SUCCEEDED( diff --git a/tools/opt/opt.cpp b/tools/opt/opt.cpp index faba610d5f..9865e13702 100644 --- a/tools/opt/opt.cpp +++ b/tools/opt/opt.cpp @@ -14,6 +14,9 @@ #include "BreakpointPrinter.h" #include "NewPMDriver.h" +#ifdef HAS_DXILCONV +#include "DxilConvPasses/InitializePasses.h" +#endif #include "llvm/ADT/Triple.h" #include "llvm/Analysis/CallGraph.h" #include "llvm/Analysis/CallGraphSCCPass.h" @@ -312,9 +315,6 @@ void initializePollyPasses(llvm::PassRegistry &Registry); #endif // HLSL Change Start -#ifdef HAS_DXILCONV -void __cdecl initializeDxilConvPasses(llvm::PassRegistry &); -#endif namespace hlsl { HRESULT SetupRegistryPassForHLSL(); } // namespace hlsl