From d6d3f02bac802550f53026e971377ce0595aa6cf Mon Sep 17 00:00:00 2001
From: Jesse Natalie <jenatali@microsoft.com>
Date: Fri, 4 Oct 2024 09:18:38 -0700
Subject: [PATCH] Fix DxilPayloadFieldAnnotation::GetPayloadFieldQualifier
 (#6942)

Fixes #6941
---
 lib/DXIL/DxilTypeSystem.cpp                   | 26 +++----------------
 tools/clang/unittests/HLSL/DxilModuleTest.cpp | 25 +++++++++++++++---
 2 files changed, 26 insertions(+), 25 deletions(-)

diff --git a/lib/DXIL/DxilTypeSystem.cpp b/lib/DXIL/DxilTypeSystem.cpp
index abffe729f2..10b62002cb 100644
--- a/lib/DXIL/DxilTypeSystem.cpp
+++ b/lib/DXIL/DxilTypeSystem.cpp
@@ -175,31 +175,13 @@ void DxilPayloadFieldAnnotation::AddPayloadFieldQualifier(
 DXIL::PayloadAccessQualifier
 DxilPayloadFieldAnnotation::GetPayloadFieldQualifier(
     DXIL::PayloadAccessShaderStage shaderStage) const {
+  if (!HasAnnotations())
+    return DXIL::PayloadAccessQualifier::ReadWrite;
 
   int bitOffset = GetBitOffsetForShaderStage(shaderStage);
-
-  // default type is always ReadWrite
-  DXIL::PayloadAccessQualifier accessType =
-      DXIL::PayloadAccessQualifier::ReadWrite;
-
-  const unsigned readBit =
-      static_cast<unsigned>(DXIL::PayloadAccessQualifier::Read);
-  const unsigned writeBit =
-      static_cast<unsigned>(DXIL::PayloadAccessQualifier::Write);
-
   unsigned accessBits = m_bitmask >> bitOffset;
-  if (accessBits & readBit) {
-    // set Read if the first bit is set
-    accessType = DXIL::PayloadAccessQualifier::Read;
-  }
-  if (accessBits & writeBit) {
-
-    // set Write only if the second bit set, if both are set set to ReadWrite
-    accessType = accessType == DXIL::PayloadAccessQualifier::ReadWrite
-                     ? DXIL::PayloadAccessQualifier::Write
-                     : DXIL::PayloadAccessQualifier::ReadWrite;
-  }
-  return accessType;
+  return (DXIL::PayloadAccessQualifier)(
+      accessBits & DXIL::PayloadAccessQualifierValidMaskPerStage);
 }
 
 bool DxilPayloadFieldAnnotation::HasAnnotations() const {
diff --git a/tools/clang/unittests/HLSL/DxilModuleTest.cpp b/tools/clang/unittests/HLSL/DxilModuleTest.cpp
index 72bf1fd303..9893127809 100644
--- a/tools/clang/unittests/HLSL/DxilModuleTest.cpp
+++ b/tools/clang/unittests/HLSL/DxilModuleTest.cpp
@@ -571,9 +571,11 @@ TEST_F(DxilModuleTest, PayloadQualifier) {
                   "{\n"
                   "  double a : read(caller, closesthit, anyhit) : "
                   "write(caller, miss, closesthit);\n"
+                  "  int b : read(caller) : write(miss);\n"
                   "};\n\n"
                   "[shader(\"miss\")]\n"
-                  "void Miss( inout Payload payload ) { payload.a = 4.2; }\n";
+                  "void Miss( inout Payload payload ) { payload.a = 4.2; "
+                  "payload.b = 1; }\n";
 
   c.Compile(shader, L"lib_6_6", arguments, {});
 
@@ -582,9 +584,9 @@ TEST_F(DxilModuleTest, PayloadQualifier) {
 
   for (auto &p : DTS.GetPayloadAnnotationMap()) {
     const DxilPayloadAnnotation &plAnnotation = *p.second;
-    for (unsigned i = 0; i < plAnnotation.GetNumFields(); ++i) {
+    {
       const DxilPayloadFieldAnnotation &fieldAnnotation =
-          plAnnotation.GetFieldAnnotation(i);
+          plAnnotation.GetFieldAnnotation(0);
       VERIFY_IS_TRUE(fieldAnnotation.HasAnnotations());
       VERIFY_ARE_EQUAL(DXIL::PayloadAccessQualifier::ReadWrite,
                        fieldAnnotation.GetPayloadFieldQualifier(
@@ -599,6 +601,23 @@ TEST_F(DxilModuleTest, PayloadQualifier) {
                        fieldAnnotation.GetPayloadFieldQualifier(
                            DXIL::PayloadAccessShaderStage::Anyhit));
     }
+    {
+      const DxilPayloadFieldAnnotation &fieldAnnotation =
+          plAnnotation.GetFieldAnnotation(1);
+      VERIFY_IS_TRUE(fieldAnnotation.HasAnnotations());
+      VERIFY_ARE_EQUAL(DXIL::PayloadAccessQualifier::Read,
+                       fieldAnnotation.GetPayloadFieldQualifier(
+                           DXIL::PayloadAccessShaderStage::Caller));
+      VERIFY_ARE_EQUAL(DXIL::PayloadAccessQualifier::NoAccess,
+                       fieldAnnotation.GetPayloadFieldQualifier(
+                           DXIL::PayloadAccessShaderStage::Closesthit));
+      VERIFY_ARE_EQUAL(DXIL::PayloadAccessQualifier::Write,
+                       fieldAnnotation.GetPayloadFieldQualifier(
+                           DXIL::PayloadAccessShaderStage::Miss));
+      VERIFY_ARE_EQUAL(DXIL::PayloadAccessQualifier::NoAccess,
+                       fieldAnnotation.GetPayloadFieldQualifier(
+                           DXIL::PayloadAccessShaderStage::Anyhit));
+    }
   }
 }