From be2ed98e0594b44b14764d6cb53cb89915e94693 Mon Sep 17 00:00:00 2001 From: Zhaoming Jiang Date: Thu, 24 Nov 2022 02:07:48 +0000 Subject: [PATCH] Dawn: Require both DXC compiler and validator version being 1.6 or higher This CL change the DXC version checking logic to get both DXC compiler and validator version, which are not necessarily identical, and require both version being 1.6 or higher to enable the use_dxc toggle. This CL also modify the src/dawn/tests/BUILD.gn and add a copy target as data_deps for "dawn_test" template, which copy DXC binaries from Windows 10 SDK 20348 to out directory, to ensure that windows trybots running dawn_end2end_tests.exe (e.g. win-dawn-rel) can access a DXC of version 1.6 and can run end-to-end tests with DXC. Bug: tint:1719 Change-Id: I39b48f3dffdf121d3749af7aa4b3d0bed1c22ea8 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/110340 Commit-Queue: Zhaoming Jiang Kokoro: Kokoro Reviewed-by: Austin Eng --- src/dawn/native/d3d12/AdapterD3D12.cpp | 9 +- src/dawn/native/d3d12/BackendD3D12.cpp | 134 +++++++++++++++++--- src/dawn/native/d3d12/BackendD3D12.h | 49 ++++++- src/dawn/native/d3d12/DeviceD3D12.cpp | 4 +- src/dawn/native/d3d12/ShaderModuleD3D12.cpp | 10 +- src/dawn/tests/BUILD.gn | 42 ++++++ 6 files changed, 215 insertions(+), 33 deletions(-) diff --git a/src/dawn/native/d3d12/AdapterD3D12.cpp b/src/dawn/native/d3d12/AdapterD3D12.cpp index a5da117891..2e749dee0f 100644 --- a/src/dawn/native/d3d12/AdapterD3D12.cpp +++ b/src/dawn/native/d3d12/AdapterD3D12.cpp @@ -142,7 +142,7 @@ MaybeError Adapter::InitializeSupportedFeaturesImpl() { mSupportedFeatures.EnableFeature(Feature::DepthClipControl); // Both Dp4a and ShaderF16 features require DXC version being 1.4 or higher - if (GetBackend()->IsDXCAvailable(1, 4)) { + if (GetBackend()->IsDXCAvailableAndVersionAtLeast(1, 4, 1, 4)) { if (mDeviceInfo.supportsDP4a) { mSupportedFeatures.EnableFeature(Feature::ChromiumExperimentalDp4a); } @@ -323,9 +323,10 @@ MaybeError Adapter::ValidateFeatureSupportedWithTogglesImpl( // D3D12. if (feature == wgpu::FeatureName::ShaderF16 || feature == wgpu::FeatureName::ChromiumExperimentalDp4a) { - DAWN_INVALID_IF( - !(userProvidedToggles.IsEnabled(Toggle::UseDXC) && mBackend->IsDXCAvailable(1, 4)), - "Feature %s requires DXC for D3D12.", GetInstance()->GetFeatureInfo(feature)->name); + DAWN_INVALID_IF(!(userProvidedToggles.IsEnabled(Toggle::UseDXC) && + mBackend->IsDXCAvailableAndVersionAtLeast(1, 4, 1, 4)), + "Feature %s requires DXC for D3D12.", + GetInstance()->GetFeatureInfo(feature)->name); } return {}; } diff --git a/src/dawn/native/d3d12/BackendD3D12.cpp b/src/dawn/native/d3d12/BackendD3D12.cpp index d8cfaa069f..ac2f5f252a 100644 --- a/src/dawn/native/d3d12/BackendD3D12.cpp +++ b/src/dawn/native/d3d12/BackendD3D12.cpp @@ -16,6 +16,7 @@ #include +#include "dawn/common/Log.h" #include "dawn/native/D3D12Backend.h" #include "dawn/native/Instance.h" #include "dawn/native/d3d12/AdapterD3D12.h" @@ -85,6 +86,51 @@ MaybeError Backend::Initialize() { mFunctions = std::make_unique(); DAWN_TRY(mFunctions->LoadFunctions()); + // Check if DXC is available and cache DXC version information + if (!mFunctions->IsDXCBinaryAvailable()) { + // DXC version information is not available if DXC binaries are not available. + mDxcVersionInfo = DxcUnavailable{"DXC binary is not available"}; + } else { + // Check the DXC version information and validate them being not lower than pre-defined + // minimum version. + AcquireDxcVersionInformation(); + + // Check that DXC version information is acquired successfully. + if (std::holds_alternative(mDxcVersionInfo)) { + const DxcVersionInfo& dxcVersionInfo = std::get(mDxcVersionInfo); + + // The required minimum version for DXC compiler and validator. + // Notes about requirement consideration: + // * DXC version 1.4 has some known issues when compiling Tint generated HLSL program, + // please + // refer to crbug.com/tint/1719 + // * Windows SDK 20348 provides DXC compiler and validator version 1.6 + // Here the minimum version requirement for DXC compiler and validator are both set + // to 1.6. + constexpr uint64_t minimumCompilerMajorVersion = 1; + constexpr uint64_t minimumCompilerMinorVersion = 6; + constexpr uint64_t minimumValidatorMajorVersion = 1; + constexpr uint64_t minimumValidatorMinorVersion = 6; + + // Check that DXC compiler and validator version are not lower than minimum. + if (dxcVersionInfo.DxcCompilerVersion < + MakeDXCVersion(minimumCompilerMajorVersion, minimumCompilerMinorVersion) || + dxcVersionInfo.DxcValidatorVersion < + MakeDXCVersion(minimumValidatorMajorVersion, minimumValidatorMinorVersion)) { + // If DXC version is lower than required minimum, set mDxcVersionInfo to + // DxcUnavailable to indicate that DXC is not available. + std::ostringstream ss; + ss << "DXC version too low: dxil.dll required version 1.6, actual version " + << (dxcVersionInfo.DxcValidatorVersion >> 32) << "." + << (dxcVersionInfo.DxcValidatorVersion & ((uint64_t(1) << 32) - 1)) + << ", dxcompiler.dll required version 1.6, actual version " + << (dxcVersionInfo.DxcCompilerVersion >> 32) << "." + << (dxcVersionInfo.DxcCompilerVersion & ((uint64_t(1) << 32) - 1)); + mDxcVersionInfo = DxcUnavailable{ss.str()}; + } + } + } + const auto instance = GetInstance(); DAWN_TRY_ASSIGN(mFactory, CreateFactory(mFunctions.get(), instance->GetBackendValidationLevel(), @@ -142,32 +188,80 @@ ComPtr Backend::GetDxcValidator() const { return mDxcValidator; } -ResultOrError Backend::GetDXCompilerVersion() { - DAWN_TRY(EnsureDxcValidator()); +void Backend::AcquireDxcVersionInformation() { + ASSERT(std::holds_alternative(mDxcVersionInfo)); - ComPtr versionInfo; - DAWN_TRY(CheckHRESULT(mDxcValidator.As(&versionInfo), - "D3D12 QueryInterface IDxcValidator to IDxcVersionInfo")); + auto tryAcquireDxcVersionInfo = [this]() -> ResultOrError { + DAWN_TRY(EnsureDxcValidator()); + DAWN_TRY(EnsureDxcCompiler()); - uint32_t compilerMajor, compilerMinor; - DAWN_TRY(CheckHRESULT(versionInfo->GetVersion(&compilerMajor, &compilerMinor), - "IDxcVersionInfo::GetVersion")); + ComPtr compilerVersionInfo; - // Pack both into a single version number. - return MakeDXCVersion(compilerMajor, compilerMinor); + DAWN_TRY(CheckHRESULT(mDxcCompiler.As(&compilerVersionInfo), + "D3D12 QueryInterface IDxcCompiler to IDxcVersionInfo")); + uint32_t compilerMajor, compilerMinor; + DAWN_TRY(CheckHRESULT(compilerVersionInfo->GetVersion(&compilerMajor, &compilerMinor), + "IDxcVersionInfo::GetVersion")); + + ComPtr validatorVersionInfo; + + DAWN_TRY(CheckHRESULT(mDxcValidator.As(&validatorVersionInfo), + "D3D12 QueryInterface IDxcValidator to IDxcVersionInfo")); + uint32_t validatorMajor, validatorMinor; + DAWN_TRY(CheckHRESULT(validatorVersionInfo->GetVersion(&validatorMajor, &validatorMinor), + "IDxcVersionInfo::GetVersion")); + + // Pack major and minor version number into a single version number. + uint64_t compilerVersion = MakeDXCVersion(compilerMajor, compilerMinor); + uint64_t validatorVersion = MakeDXCVersion(validatorMajor, validatorMinor); + return DxcVersionInfo{compilerVersion, validatorVersion}; + }; + + auto dxcVersionInfoOrError = tryAcquireDxcVersionInfo(); + + if (dxcVersionInfoOrError.IsSuccess()) { + // Cache the DXC version information. + mDxcVersionInfo = dxcVersionInfoOrError.AcquireSuccess(); + } else { + // Error occurs when acquiring DXC version information, set the cache to unavailable and + // record the error message. + std::string errorMessage = dxcVersionInfoOrError.AcquireError()->GetFormattedMessage(); + dawn::ErrorLog() << errorMessage; + mDxcVersionInfo = DxcUnavailable{errorMessage}; + } +} + +// Return both DXC compiler and DXC validator version, assert that DXC version information is +// acquired succesfully. +DxcVersionInfo Backend::GetDxcVersion() const { + ASSERT(std::holds_alternative(mDxcVersionInfo)); + return DxcVersionInfo(std::get(mDxcVersionInfo)); } // Return true if and only if DXC binary is avaliable, and the DXC version is validated to -// be no older than given minimum version. -bool Backend::IsDXCAvailable(uint64_t minimumMajorVersion, uint64_t minimumMinorVersion) { - if (mFunctions->IsDXCBinaryAvailable()) { - auto versionOrError = GetDXCompilerVersion(); - if (versionOrError.IsSuccess()) { - // Validate the DXC version - auto version = versionOrError.AcquireSuccess(); - if (version >= MakeDXCVersion(minimumMajorVersion, minimumMinorVersion)) { - return true; - } +// be no older than a pre-defined minimum version. +bool Backend::IsDXCAvailable() const { + // mDxcVersionInfo hold DxcVersionInfo instead of DxcUnavailable if and only if DXC binaries and + // version are validated in `Initialize`. + return std::holds_alternative(mDxcVersionInfo); +} + +// Return true if and only if IsDXCAvailable() return true, and the DXC compiler and validator +// version are validated to be no older than the minimium version given in parameter. +bool Backend::IsDXCAvailableAndVersionAtLeast(uint64_t minimumCompilerMajorVersion, + uint64_t minimumCompilerMinorVersion, + uint64_t minimumValidatorMajorVersion, + uint64_t minimumValidatorMinorVersion) const { + // mDxcVersionInfo hold DxcVersionInfo instead of DxcUnavailable if and only if DXC binaries and + // version are validated in `Initialize`. + if (std::holds_alternative(mDxcVersionInfo)) { + const DxcVersionInfo& dxcVersionInfo = std::get(mDxcVersionInfo); + // Check that DXC compiler and validator version are not lower than given requirements. + if (dxcVersionInfo.DxcCompilerVersion >= + MakeDXCVersion(minimumCompilerMajorVersion, minimumCompilerMinorVersion) && + dxcVersionInfo.DxcValidatorVersion >= + MakeDXCVersion(minimumValidatorMajorVersion, minimumValidatorMinorVersion)) { + return true; } } return false; diff --git a/src/dawn/native/d3d12/BackendD3D12.h b/src/dawn/native/d3d12/BackendD3D12.h index 7cd79ec13b..4d0184a22b 100644 --- a/src/dawn/native/d3d12/BackendD3D12.h +++ b/src/dawn/native/d3d12/BackendD3D12.h @@ -16,6 +16,8 @@ #define SRC_DAWN_NATIVE_D3D12_BACKENDD3D12_H_ #include +#include +#include #include #include "dawn/native/BackendConnection.h" @@ -26,6 +28,21 @@ namespace dawn::native::d3d12 { class PlatformFunctions; +// DxcVersionInfo holds both DXC compiler (dxcompiler.dll) version and DXC validator (dxil.dll) +// version, which are not necessarily identical. Both are in uint64_t type, as the result of +// MakeDXCVersion. +struct DxcVersionInfo { + uint64_t DxcCompilerVersion; + uint64_t DxcValidatorVersion; +}; + +// If DXC version information is not avaliable due to no DXC binary or error occurs when acquiring +// version, DxcUnavailable indicates the version information being unavailable and holds the +// detailed error information. +struct DxcUnavailable { + std::string ErrorMessage; +}; + class Backend : public BackendConnection { public: explicit Backend(InstanceBase* instance); @@ -40,11 +57,21 @@ class Backend : public BackendConnection { ComPtr GetDxcLibrary() const; ComPtr GetDxcCompiler() const; ComPtr GetDxcValidator() const; - ResultOrError GetDXCompilerVersion(); - // Return true if and only if DXC binary is avaliable, and the DXC version is validated to - // be no older than given minimium version. - bool IsDXCAvailable(uint64_t minimumMajorVersion, uint64_t minimumMinorVersion); + // Return true if and only if DXC binary is avaliable, and the DXC compiler and validator + // version are validated to be no older than a specific minimium version, currently 1.6. + bool IsDXCAvailable() const; + + // Return true if and only if mIsDXCAvailable is true, and the DXC compiler and validator + // version are validated to be no older than the minimium version given in parameter. + bool IsDXCAvailableAndVersionAtLeast(uint64_t minimumCompilerMajorVersion, + uint64_t minimumCompilerMinorVersion, + uint64_t minimumValidatorMajorVersion, + uint64_t minimumValidatorMinorVersion) const; + + // Return the DXC version information cached in mDxcVersionInformation, assert that the version + // information is valid. Must be called after ensuring `IsDXCAvailable()` return true. + DxcVersionInfo GetDxcVersion() const; const PlatformFunctions* GetFunctions() const; @@ -53,6 +80,10 @@ class Backend : public BackendConnection { const AdapterDiscoveryOptionsBase* optionsBase) override; private: + // Acquiring DXC version information and store the result in mDxcVersionInfo. This function + // should be called only once, during startup in `Initialize`. + void AcquireDxcVersionInformation(); + // Keep mFunctions as the first member so that in the destructor it is freed last. Otherwise // the D3D12 DLLs are unloaded before we are done using them. std::unique_ptr mFunctions; @@ -60,6 +91,16 @@ class Backend : public BackendConnection { ComPtr mDxcLibrary; ComPtr mDxcCompiler; ComPtr mDxcValidator; + + // DXC binaries and DXC version information are checked when start up in `Initialize`. There are + // two possible states: + // 1. The DXC binary is not available, or error occurs when checking the version information + // and therefore no DXC version information available, or the DXC version is lower than + // requested minumum and therefore DXC is not available, represented by DxcUnavailable + // in which a error message is held; + // 3. The DXC version information is acquired successfully and validated not lower than + // requested minimum, stored in DxcVersionInfo. + std::variant mDxcVersionInfo; }; } // namespace dawn::native::d3d12 diff --git a/src/dawn/native/d3d12/DeviceD3D12.cpp b/src/dawn/native/d3d12/DeviceD3D12.cpp index b0076491c2..56ce607f12 100644 --- a/src/dawn/native/d3d12/DeviceD3D12.cpp +++ b/src/dawn/native/d3d12/DeviceD3D12.cpp @@ -235,9 +235,7 @@ ComPtr Device::GetFactory() const { } MaybeError Device::ApplyUseDxcToggle() { - // Require DXC version 1.4 or higher to enable using DXC, as DXC 1.2 have some known issues when - // compiling Tint generated HLSL program. Please refer to crbug.com/tint/1719. - if (!ToBackend(GetAdapter())->GetBackend()->IsDXCAvailable(1, 4)) { + if (!ToBackend(GetAdapter())->GetBackend()->IsDXCAvailable()) { ForceSetToggle(Toggle::UseDXC, false); } diff --git a/src/dawn/native/d3d12/ShaderModuleD3D12.cpp b/src/dawn/native/d3d12/ShaderModuleD3D12.cpp index 68a6f72a9e..ad56d08b5e 100644 --- a/src/dawn/native/d3d12/ShaderModuleD3D12.cpp +++ b/src/dawn/native/d3d12/ShaderModuleD3D12.cpp @@ -494,11 +494,17 @@ ResultOrError ShaderModule::Compile( req.bytecode.compileFlags = compileFlags; if (device->IsToggleEnabled(Toggle::UseDXC)) { + // If UseDXC toggle are not forced to be disable, DXC should have been validated to be + // available. + ASSERT(ToBackend(device->GetAdapter())->GetBackend()->IsDXCAvailable()); + // We can get the DXC version information since IsDXCAvailable() is true. + DxcVersionInfo dxcVersionInfo = + ToBackend(device->GetAdapter())->GetBackend()->GetDxcVersion(); + req.bytecode.compiler = Compiler::DXC; req.bytecode.dxcLibrary = device->GetDxcLibrary().Get(); req.bytecode.dxcCompiler = device->GetDxcCompiler().Get(); - DAWN_TRY_ASSIGN(req.bytecode.compilerVersion, - ToBackend(device->GetAdapter())->GetBackend()->GetDXCompilerVersion()); + req.bytecode.compilerVersion = dxcVersionInfo.DxcCompilerVersion; req.bytecode.dxcShaderProfile = device->GetDeviceInfo().shaderProfiles[stage]; } else { req.bytecode.compiler = Compiler::FXC; diff --git a/src/dawn/tests/BUILD.gn b/src/dawn/tests/BUILD.gn index c1e0137432..80bfe3fcd5 100644 --- a/src/dawn/tests/BUILD.gn +++ b/src/dawn/tests/BUILD.gn @@ -114,6 +114,37 @@ if (build_with_chromium) { } } +############################################################################### +# Copy target for DXC +############################################################################### + +# When building with chromium under Windows, we need to copy the DXC from +# Windows SDK into build folder, in order to ensure a DXC of verison 1.6 is +# avaliable when running end-to-end tests in Chromium. Note that currently DXC +# only provided for x86 and x64 in Windows SDK 20348. +if (build_with_chromium && is_win && + (target_cpu == "x64" || target_cpu == "x86")) { + # The windows_sdk_path is acquired in visual_studio_version.gni. + import("//build/config/win/visual_studio_version.gni") + + # Windows 10 SDK version 10.0.20348.0 is required for building Chromium, and + # it provided DXC compiler and validator both of v1.6. + assert(defined(windows_sdk_path)) + assert(windows_sdk_path != "") + + # Declare a copy target to copy DXC binary from Windows SDK for running Dawn + # end-to-end test in Chromium. + copy("copy_dxc_binary") { + sources = [ + "$windows_sdk_path/bin/10.0.20348.0/$target_cpu/dxc.exe", + "$windows_sdk_path/bin/10.0.20348.0/$target_cpu/dxcompiler.dll", + "$windows_sdk_path/bin/10.0.20348.0/$target_cpu/dxil.dll", + ] + + outputs = [ "$root_out_dir/{{source_file_part}}" ] + } +} + ############################################################################### # Dawn test template ############################################################################### @@ -126,6 +157,17 @@ template("dawn_test") { } configs += [ "${dawn_root}/src/dawn/common:internal_config" ] + + # Running Dawn tests within Chromium in Windows may require DXC, copy DXC + # binary from Windows SDK. + if (build_with_chromium && is_win && + (target_cpu == "x64" || target_cpu == "x86")) { + if (defined(data_deps)) { + data_deps += [ ":copy_dxc_binary" ] + } else { + data_deps = [ ":copy_dxc_binary" ] + } + } } }