From 22bb0786b68e38f21cd16e98f97aad8ed1a0a058 Mon Sep 17 00:00:00 2001 From: Tex Riddell Date: Tue, 20 Feb 2024 17:58:45 -0800 Subject: [PATCH] Fix downlevel validator compatibility with DXR 1.1 flag (#6333) It turns out that in the prior validator version, if a subobject required DXR 1.1, the DXR 1.1 flag would be set on each function in RDAT output, as well as the global flags. It didn't appear this was the case through a D3DReflect test because that goes through disassembly to assembly step, where subobjects are lost because they aren't in the llvm IR. The previous change assumed this was not the case when the subobjects were removed, but this removal occurs after the RDATWriter constructor, which does the full RDAT serialization since size if required right away. This restores the detection code and hooks it into DxilModule::ComputeShaderCompatInfo when validator version is in range [1.5, 1.7]. DXR 1.1 was introduced in 1.5, and we no longer tag every function as requiring DXR 1.1 based on subobjects in 1.8. At the moment, there is no way to CHECK the subobject RDAT in D3DReflect because they get stripped from the module (even reflection and debug modules) before serialization, and the test path for D3DReflect goes through a disassemble/re-assemble step between the prior stage and the D3DReflect stage. Fixes #6321 (really). --- lib/DXIL/DxilModule.cpp | 28 +++++++++++++++++++ lib/HLSL/DxilValidation.cpp | 14 ++++++++++ .../subobjects_raytracingPipelineConfig1.hlsl | 8 +++--- 3 files changed, 46 insertions(+), 4 deletions(-) diff --git a/lib/DXIL/DxilModule.cpp b/lib/DXIL/DxilModule.cpp index f03907faa6..98531b80b1 100644 --- a/lib/DXIL/DxilModule.cpp +++ b/lib/DXIL/DxilModule.cpp @@ -2191,6 +2191,27 @@ static void AdjustMinimumShaderModelAndFlags(const DxilFunctionProps *props, DXIL::UpdateToMaxOfVersions(minMajor, minMinor, 6, 1); } +static bool RequiresRaytracingTier1_1(const DxilSubobjects *pSubobjects) { + if (!pSubobjects) + return false; + for (const auto &it : pSubobjects->GetSubobjects()) { + switch (it.second->GetKind()) { + case DXIL::SubobjectKind::RaytracingPipelineConfig1: + return true; + case DXIL::SubobjectKind::StateObjectConfig: { + uint32_t configFlags; + if (it.second->GetStateObjectConfig(configFlags) && + ((configFlags & + (unsigned)DXIL::StateObjectFlags::AllowStateObjectAdditions) != 0)) + return true; + } break; + default: + break; + } + } + return false; +} + void DxilModule::ComputeShaderCompatInfo() { m_FuncToShaderCompat.clear(); @@ -2198,6 +2219,11 @@ void DxilModule::ComputeShaderCompatInfo() { bool dxil18Plus = DXIL::CompareVersions(m_ValMajor, m_ValMinor, 1, 8) >= 0; bool dxil19Plus = DXIL::CompareVersions(m_ValMajor, m_ValMinor, 1, 9) >= 0; + // Prior to validator version 1.8, DXR 1.1 flag was set on every function + // if subobjects contained any DXR 1.1 subobjects. + bool setDXR11OnAllFunctions = + dxil15Plus && !dxil18Plus && RequiresRaytracingTier1_1(GetSubobjects()); + // Initialize worklist with functions that have callers SmallSetVector worklist; @@ -2212,6 +2238,8 @@ void DxilModule::ComputeShaderCompatInfo() { // Insert or lookup info ShaderCompatInfo &info = m_FuncToShaderCompat[&function]; info.shaderFlags = ShaderFlags::CollectShaderFlags(&function, this); + if (setDXR11OnAllFunctions) + info.shaderFlags.SetRaytracingTier1_1(true); } else if (!function.isIntrinsic() && function.getLinkage() == llvm::GlobalValue::LinkageTypes::ExternalLinkage && diff --git a/lib/HLSL/DxilValidation.cpp b/lib/HLSL/DxilValidation.cpp index 0a7eef7a91..b75ce4d62a 100644 --- a/lib/HLSL/DxilValidation.cpp +++ b/lib/HLSL/DxilValidation.cpp @@ -4487,6 +4487,20 @@ static void ValidateResources(ValidationContext &ValCtx) { static void ValidateShaderFlags(ValidationContext &ValCtx) { ShaderFlags calcFlags; ValCtx.DxilMod.CollectShaderFlagsForModule(calcFlags); + + // Special case for validator version prior to 1.8. + // If DXR 1.1 flag is set, but our computed flags do not have this set, then + // this is due to prior versions setting the flag based on DXR 1.1 subobjects, + // which are gone by this point. Set the flag and the rest should match. + unsigned valMajor, valMinor; + ValCtx.DxilMod.GetValidatorVersion(valMajor, valMinor); + if (DXIL::CompareVersions(valMajor, valMinor, 1, 5) >= 0 && + DXIL::CompareVersions(valMajor, valMinor, 1, 8) < 0 && + ValCtx.DxilMod.m_ShaderFlags.GetRaytracingTier1_1() && + !calcFlags.GetRaytracingTier1_1()) { + calcFlags.SetRaytracingTier1_1(true); + } + const uint64_t mask = ShaderFlags::GetShaderFlagsRawForCollection(); uint64_t declaredFlagsRaw = ValCtx.DxilMod.m_ShaderFlags.GetShaderFlagsRaw(); uint64_t calcFlagsRaw = calcFlags.GetShaderFlagsRaw(); diff --git a/tools/clang/test/HLSLFileCheck/shader_targets/raytracing/subobjects_raytracingPipelineConfig1.hlsl b/tools/clang/test/HLSLFileCheck/shader_targets/raytracing/subobjects_raytracingPipelineConfig1.hlsl index a9a1bb9be1..cc54d317fa 100644 --- a/tools/clang/test/HLSLFileCheck/shader_targets/raytracing/subobjects_raytracingPipelineConfig1.hlsl +++ b/tools/clang/test/HLSLFileCheck/shader_targets/raytracing/subobjects_raytracingPipelineConfig1.hlsl @@ -1,9 +1,9 @@ -// RUN: %dxilver 1.5 | %dxc -T lib_6_3 %s | FileCheck %s +// RUN: %dxilver 1.5 | %dxc -T lib_6_3 -validator-version 1.5 %s | FileCheck %s -check-prefixes=CHECK,CHECK15 +// RUN: %dxilver 1.8 | %dxc -T lib_6_3 -validator-version 1.8 %s | FileCheck %s -check-prefixes=CHECK,CHECK18 // RaytracingTier1_1 flag should not be set on the module based on subobjects. -// This is not even set for compatibility with validator 1.7 because that -// validator did not validate the flags. -// CHECK-NOT: Raytracing tier 1.1 features +// CHECK18-NOT: Raytracing tier 1.1 features +// CHECK15: Raytracing tier 1.1 features // CHECK: ; GlobalRootSignature grs = { <48 bytes> }; // CHECK: ; StateObjectConfig soc = { STATE_OBJECT_FLAG_ALLOW_LOCAL_DEPENDENCIES_ON_EXTERNAL_DEFINITIONS | STATE_OBJECT_FLAG_ALLOW_STATE_OBJECT_ADDITIONS };