From 9fc7a77609891bf66792cce47f4ad483935bf310 Mon Sep 17 00:00:00 2001 From: Zhaoming Jiang Date: Mon, 17 Oct 2022 08:39:06 +0000 Subject: [PATCH] Always validate DXC version and forbid DXC older than 1.4 This CL add a `Backend::IsDXCAvailable` method that check not only the DXC binary is available but also its version is no older than a given minimum version, and use this function to replace all previous `PlatformFunctions::IsDXCAvailable` to ensure that we always check the DXC version. By giving the minimum version 1.4, this CL also forbid using DXC older than 1.4. Issue: tint:1719 Change-Id: I6ab0a3791ac734c4e8b13570c55194573f111e61 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/105900 Reviewed-by: Ben Clayton Reviewed-by: Corentin Wallez Commit-Queue: Zhaoming Jiang Kokoro: Kokoro --- src/dawn/native/d3d12/AdapterD3D12.cpp | 20 ++++++++------------ src/dawn/native/d3d12/BackendD3D12.cpp | 16 ++++++++++++++++ src/dawn/native/d3d12/BackendD3D12.h | 4 ++++ src/dawn/native/d3d12/DeviceD3D12.cpp | 4 +++- src/dawn/native/d3d12/PlatformFunctions.cpp | 4 +++- src/dawn/native/d3d12/PlatformFunctions.h | 2 +- 6 files changed, 35 insertions(+), 15 deletions(-) diff --git a/src/dawn/native/d3d12/AdapterD3D12.cpp b/src/dawn/native/d3d12/AdapterD3D12.cpp index 8140e9f01f..3f61a40dce 100644 --- a/src/dawn/native/d3d12/AdapterD3D12.cpp +++ b/src/dawn/native/d3d12/AdapterD3D12.cpp @@ -140,13 +140,9 @@ MaybeError Adapter::InitializeSupportedFeaturesImpl() { mSupportedFeatures.EnableFeature(Feature::RG11B10UfloatRenderable); mSupportedFeatures.EnableFeature(Feature::DepthClipControl); - if (GetBackend()->GetFunctions()->IsDXCAvailable()) { - uint64_t dxcVersion = 0; - DAWN_TRY_ASSIGN(dxcVersion, GetBackend()->GetDXCompilerVersion()); - constexpr uint64_t kLeastMajorVersionForDP4a = 1; - constexpr uint64_t kLeastMinorVersionForDP4a = 4; - if (mDeviceInfo.supportsDP4a && - dxcVersion >= MakeDXCVersion(kLeastMajorVersionForDP4a, kLeastMinorVersionForDP4a)) { + // Both Dp4a and ShaderF16 features require DXC version being 1.4 or higher + if (GetBackend()->IsDXCAvailable(1, 4)) { + if (mDeviceInfo.supportsDP4a) { mSupportedFeatures.EnableFeature(Feature::ChromiumExperimentalDp4a); } if (mDeviceInfo.supportsShaderF16) { @@ -322,13 +318,13 @@ MaybeError Adapter::InitializeSupportedLimitsImpl(CombinedLimits* limits) { MaybeError Adapter::ValidateFeatureSupportedWithTogglesImpl( wgpu::FeatureName feature, const TripleStateTogglesSet& userProvidedToggles) { - // shader-f16 feature and chromium-experimental-dp4a feature require DXC for D3D12. + // shader-f16 feature and chromium-experimental-dp4a feature require DXC 1.4 or higher for + // D3D12. if (feature == wgpu::FeatureName::ShaderF16 || feature == wgpu::FeatureName::ChromiumExperimentalDp4a) { - DAWN_INVALID_IF(!(userProvidedToggles.IsEnabled(Toggle::UseDXC) && - mBackend->GetFunctions()->IsDXCAvailable()), - "Feature %s requires DXC for D3D12.", - GetInstance()->GetFeatureInfo(feature)->name); + DAWN_INVALID_IF( + !(userProvidedToggles.IsEnabled(Toggle::UseDXC) && mBackend->IsDXCAvailable(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 3d0b13f491..d8cfaa069f 100644 --- a/src/dawn/native/d3d12/BackendD3D12.cpp +++ b/src/dawn/native/d3d12/BackendD3D12.cpp @@ -157,6 +157,22 @@ ResultOrError Backend::GetDXCompilerVersion() { return MakeDXCVersion(compilerMajor, compilerMinor); } +// 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; + } + } + } + return false; +} + const PlatformFunctions* Backend::GetFunctions() const { return mFunctions.get(); } diff --git a/src/dawn/native/d3d12/BackendD3D12.h b/src/dawn/native/d3d12/BackendD3D12.h index 1bf1ead583..7cd79ec13b 100644 --- a/src/dawn/native/d3d12/BackendD3D12.h +++ b/src/dawn/native/d3d12/BackendD3D12.h @@ -42,6 +42,10 @@ class Backend : public BackendConnection { 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); + const PlatformFunctions* GetFunctions() const; std::vector> DiscoverDefaultAdapters() override; diff --git a/src/dawn/native/d3d12/DeviceD3D12.cpp b/src/dawn/native/d3d12/DeviceD3D12.cpp index 840351c104..835595a3b0 100644 --- a/src/dawn/native/d3d12/DeviceD3D12.cpp +++ b/src/dawn/native/d3d12/DeviceD3D12.cpp @@ -235,7 +235,9 @@ ComPtr Device::GetFactory() const { } MaybeError Device::ApplyUseDxcToggle() { - if (!ToBackend(GetAdapter())->GetBackend()->GetFunctions()->IsDXCAvailable()) { + // 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)) { ForceSetToggle(Toggle::UseDXC, false); } diff --git a/src/dawn/native/d3d12/PlatformFunctions.cpp b/src/dawn/native/d3d12/PlatformFunctions.cpp index 70b66d8efb..1a3f3fe1da 100644 --- a/src/dawn/native/d3d12/PlatformFunctions.cpp +++ b/src/dawn/native/d3d12/PlatformFunctions.cpp @@ -244,7 +244,9 @@ bool PlatformFunctions::IsPIXEventRuntimeLoaded() const { return mPIXEventRuntimeLib.Valid(); } -bool PlatformFunctions::IsDXCAvailable() const { +// Use Backend::IsDXCAvaliable if possible, which also check the DXC is no older than a given +// version +bool PlatformFunctions::IsDXCBinaryAvailable() const { return mDXILLib.Valid() && mDXCompilerLib.Valid(); } diff --git a/src/dawn/native/d3d12/PlatformFunctions.h b/src/dawn/native/d3d12/PlatformFunctions.h index 6d2e2225a9..fca1e51ba0 100644 --- a/src/dawn/native/d3d12/PlatformFunctions.h +++ b/src/dawn/native/d3d12/PlatformFunctions.h @@ -36,7 +36,7 @@ class PlatformFunctions { MaybeError LoadFunctions(); bool IsPIXEventRuntimeLoaded() const; - bool IsDXCAvailable() const; + bool IsDXCBinaryAvailable() const; // Functions from d3d12.dll PFN_D3D12_CREATE_DEVICE d3d12CreateDevice = nullptr;