From 383a08f47ab7667becbe194e513f8e0a32f8b4ed Mon Sep 17 00:00:00 2001
From: Tex Riddell <texr@microsoft.com>
Date: Fri, 16 Feb 2024 15:16:14 -0800
Subject: [PATCH] Don't set RaytracingTier1_1 based on subobjects; enable flag
 validation (#6320)

In validator version 1.7 and below, the RaytracingTier1_1 module flag
was set on every function if any StateObjectConfig subobject had the
AllowStateObjectAdditions flag set. This was incorrect, as the
requirement is validated in the runtime based on the use of the
subobject instead. Subobjects are removed from the module and placed in
RDAT during container serialization, so the requirement would be lost
when recomputing the flags in validation. This didn't break validation
because flag validation was completely disabled for libraries!

This change fixes this problem, and allows DxilValidation to validate
ShaderFlags because they will no longer mismatch due to this issue.
Running CollectShaderFlagsForModule is also necessary for collecting
per-function shader flags which will be used by call graph validation in
a subsequent change, so enabling flag validation unblocks that as well.

Fixes #6321

(cherry picked from commit fafbc4272540c09cd070ca8a7a8dcda2788bc108)
---
 lib/DXIL/DxilModule.cpp                       | 25 -------------------
 lib/HLSL/DxilValidation.cpp                   |  4 ---
 .../subobjects_raytracingPipelineConfig1.hlsl | 12 ++++++---
 3 files changed, 9 insertions(+), 32 deletions(-)

diff --git a/lib/DXIL/DxilModule.cpp b/lib/DXIL/DxilModule.cpp
index 68133f7393..61813e9508 100644
--- a/lib/DXIL/DxilModule.cpp
+++ b/lib/DXIL/DxilModule.cpp
@@ -288,27 +288,6 @@ unsigned DxilModule::GetGlobalFlags() const {
   return Flags;
 }
 
-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::CollectShaderFlagsForModule(ShaderFlags &Flags) {
   ComputeShaderCompatInfo();
   for (auto &itInfo : m_FuncToShaderCompat)
@@ -380,10 +359,6 @@ void DxilModule::CollectShaderFlagsForModule(ShaderFlags &Flags) {
   bool hasCSRawAndStructuredViaShader4X =
       hasRawAndStructuredBuffer && m_pSM->GetMajor() == 4 && m_pSM->IsCS();
   Flags.SetCSRawAndStructuredViaShader4X(hasCSRawAndStructuredViaShader4X);
-
-  if (!Flags.GetRaytracingTier1_1()) {
-    Flags.SetRaytracingTier1_1(RequiresRaytracingTier1_1(GetSubobjects()));
-  }
 }
 
 void DxilModule::CollectShaderFlagsForModule() {
diff --git a/lib/HLSL/DxilValidation.cpp b/lib/HLSL/DxilValidation.cpp
index 8fe34fb281..e360a54ee3 100644
--- a/lib/HLSL/DxilValidation.cpp
+++ b/lib/HLSL/DxilValidation.cpp
@@ -4448,10 +4448,6 @@ static void ValidateResources(ValidationContext &ValCtx) {
 }
 
 static void ValidateShaderFlags(ValidationContext &ValCtx) {
-  // TODO: validate flags foreach entry.
-  if (ValCtx.isLibProfile)
-    return;
-
   ShaderFlags calcFlags;
   ValCtx.DxilMod.CollectShaderFlagsForModule(calcFlags);
   const uint64_t mask = ShaderFlags::GetShaderFlagsRawForCollection();
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 b763f8b5c6..a9a1bb9be1 100644
--- a/tools/clang/test/HLSLFileCheck/shader_targets/raytracing/subobjects_raytracingPipelineConfig1.hlsl
+++ b/tools/clang/test/HLSLFileCheck/shader_targets/raytracing/subobjects_raytracingPipelineConfig1.hlsl
@@ -1,5 +1,10 @@
 // RUN: %dxilver 1.5 | %dxc -T lib_6_3 %s | FileCheck %s
 
+// 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
+
 // CHECK: ; GlobalRootSignature grs = { <48 bytes> };
 // CHECK: ; StateObjectConfig soc = { STATE_OBJECT_FLAG_ALLOW_LOCAL_DEPENDENCIES_ON_EXTERNAL_DEFINITIONS | STATE_OBJECT_FLAG_ALLOW_STATE_OBJECT_ADDITIONS };
 // CHECK: ; LocalRootSignature lrs = { <48 bytes> };
@@ -27,6 +32,7 @@ RaytracingPipelineConfig1 rpc2 = {32, RAYTRACING_PIPELINE_FLAG_NONE };
 TriangleHitGroup trHitGt = {"a", "b"};
 ProceduralPrimitiveHitGroup ppHitGt = { "a", "b", "c"};
 
-int main(int i : INDEX) : SV_Target {
-  return 1;
-}
\ No newline at end of file
+// DXR entry point to ensure RDAT flags match during validation.
+[shader("raygeneration")]
+void main(void) {
+}