Attach resource attributes to handle within record, instead of record

[bob80905]

This PR attaches the resource attributes, HLSLResourceAttr and HLSLResourceClassAttr, to the handle contained within such resource classes like RWBuffer. CodeGen will now search for fields within HLSL resource decls, and emit previous data based on attributes on the handle member. An AST-dump test was added to verify that the resource attribute is attached to the handle within the resource record decl.
Fixes #98556

[llvmbot]

@llvm/pr-subscribers-clang-codegen
@llvm/pr-subscribers-clang

@llvm/pr-subscribers-hlsl

Author: Joshua Batista (bob80905)

Changes

This PR attaches the resource attributes, HLSLResourceAttr and HLSLResourceClassAttr, to the handle contained within such resource classes like RWBuffer. CodeGen will now search for fields within HLSL resource decls, and emit previous data based on attributes on the handle member. An AST-dump test was added to verify that the resource attribute is attached to the handle within the resource record decl.
Fixes #98556

---

Full diff: https://github.com/llvm/llvm-project/pull/101433.diff

3 Files Affected:

• (modified) clang/lib/CodeGen/CGHLSLRuntime.cpp (+14-10)
• (modified) clang/lib/Sema/HLSLExternalSemaSource.cpp (+12-16)
• (added) clang/test/ParserHLSL/hlsl_resource_handle_attrs.hlsl (+14)

diff --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp b/clang/lib/CodeGen/CGHLSLRuntime.cpp
index 6a6aff594fb0f..a475d46966969 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -280,18 +280,22 @@ void CGHLSLRuntime::annotateHLSLResource(const VarDecl *D, GlobalVariable *GV) {
   const auto *RD = Ty->getAsCXXRecordDecl();
   if (!RD)
     return;
-  const auto *HLSLResAttr = RD->getAttr<HLSLResourceAttr>();
-  const auto *HLSLResClassAttr = RD->getAttr<HLSLResourceClassAttr>();
-  if (!HLSLResAttr || !HLSLResClassAttr)
-    return;
+  // the resource related attributes are on the handle member 
+  // inside the record decl
+  for (auto *FD : RD->fields()) {
+    const auto *HLSLResAttr = FD->getAttr<HLSLResourceAttr>();
+    const auto *HLSLResClassAttr = FD->getAttr<HLSLResourceClassAttr>();
+    if (!HLSLResAttr || !HLSLResClassAttr)
+      continue;
 
-  llvm::hlsl::ResourceClass RC = HLSLResClassAttr->getResourceClass();
-  llvm::hlsl::ResourceKind RK = HLSLResAttr->getResourceKind();
-  bool IsROV = HLSLResAttr->getIsROV();
-  llvm::hlsl::ElementType ET = calculateElementType(CGM.getContext(), Ty);
+    llvm::hlsl::ResourceClass RC = HLSLResClassAttr->getResourceClass();
+    llvm::hlsl::ResourceKind RK = HLSLResAttr->getResourceKind();
+    bool IsROV = HLSLResAttr->getIsROV();
+    llvm::hlsl::ElementType ET = calculateElementType(CGM.getContext(), Ty);
 
-  BufferResBinding Binding(D->getAttr<HLSLResourceBindingAttr>());
-  addBufferResourceAnnotation(GV, RC, RK, IsROV, ET, Binding);
+    BufferResBinding Binding(D->getAttr<HLSLResourceBindingAttr>());
+    addBufferResourceAnnotation(GV, RC, RK, IsROV, ET, Binding);
+	}
 }
 
 CGHLSLRuntime::BufferResBinding::BufferResBinding(
diff --git a/clang/lib/Sema/HLSLExternalSemaSource.cpp b/clang/lib/Sema/HLSLExternalSemaSource.cpp
index ca88d138aef5d..e651f9cd967f5 100644
--- a/clang/lib/Sema/HLSLExternalSemaSource.cpp
+++ b/clang/lib/Sema/HLSLExternalSemaSource.cpp
@@ -101,8 +101,9 @@ struct BuiltinTypeDeclBuilder {
     return *this;
   }
 
-  BuiltinTypeDeclBuilder &
-  addHandleMember(AccessSpecifier Access = AccessSpecifier::AS_private) {
+  BuiltinTypeDeclBuilder &addHandleMember(
+      ResourceClass RC, ResourceKind RK,
+      bool IsROV, AccessSpecifier Access = AccessSpecifier::AS_private) {
     if (Record->isCompleteDefinition())
       return *this;
     QualType Ty = Record->getASTContext().VoidPtrTy;
@@ -112,17 +113,13 @@ struct BuiltinTypeDeclBuilder {
         Ty = Record->getASTContext().getPointerType(
             QualType(TTD->getTypeForDecl(), 0));
     }
-    return addMemberVariable("h", Ty, Access);
-  }
-
-  BuiltinTypeDeclBuilder &annotateHLSLResource(ResourceClass RC,
-                                               ResourceKind RK, bool IsROV) {
-    if (Record->isCompleteDefinition())
-      return *this;
-    Record->addAttr(
-        HLSLResourceClassAttr::CreateImplicit(Record->getASTContext(), RC));
-    Record->addAttr(
-        HLSLResourceAttr::CreateImplicit(Record->getASTContext(), RK, IsROV));
+    // add handle member
+    addMemberVariable("h", Ty, Access);
+    // add resource attributes to handle
+    auto *FD = Fields["h"];
+    FD->addAttr(HLSLResourceClassAttr::CreateImplicit(FD->getASTContext(), RC));
+    FD->addAttr(
+        HLSLResourceAttr::CreateImplicit(FD->getASTContext(), RK, IsROV));
     return *this;
   }
 
@@ -489,9 +486,8 @@ static BuiltinTypeDeclBuilder setupBufferType(CXXRecordDecl *Decl, Sema &S,
                                               ResourceClass RC, ResourceKind RK,
                                               bool IsROV) {
   return BuiltinTypeDeclBuilder(Decl)
-      .addHandleMember()
-      .addDefaultHandleConstructor(S, RC)
-      .annotateHLSLResource(RC, RK, IsROV);
+      .addHandleMember(RC, RK, IsROV)
+      .addDefaultHandleConstructor(S, RC);
 }
 
 void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
diff --git a/clang/test/ParserHLSL/hlsl_resource_handle_attrs.hlsl b/clang/test/ParserHLSL/hlsl_resource_handle_attrs.hlsl
new file mode 100644
index 0000000000000..6b7bcbc35b8f8
--- /dev/null
+++ b/clang/test/ParserHLSL/hlsl_resource_handle_attrs.hlsl
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -ast-dump -o - %s | FileCheck %s
+
+// CHECK: -ClassTemplateDecl 0x{{[0-9a-f]+}} <<invalid sloc>> <invalid sloc> implicit RWBuffer
+// CHECK: -CXXRecordDecl 0x{{[0-9a-f]+}} <<invalid sloc>> <invalid sloc> implicit class RWBuffer definition
+// CHECK: -FieldDecl 0x{{[0-9a-f]+}} <<invalid sloc>> <invalid sloc> implicit h 'element_type *'
+// CHECK: -HLSLResourceClassAttr 0x{{[0-9a-f]+}} <<invalid sloc>> Implicit UAV
+// CHECK: -HLSLResourceAttr 0x{{[0-9a-f]+}} <<invalid sloc>> Implicit TypedBuffer
+RasterizerOrderedBuffer<vector<float, 4> > BufferArray3[4] : register(u4, space1);
+
+// CHECK: -ClassTemplateSpecializationDecl 0x{{[0-9a-f]+}} <<invalid sloc>> <invalid sloc> class RWBuffer definition implicit_instantiation
+// CHECK: -FieldDecl 0x{{[0-9a-f]+}} <<invalid sloc>> <invalid sloc> implicit referenced h 'float *'
+// CHECK: -HLSLResourceClassAttr 0x{{[0-9a-f]+}} <<invalid sloc>> Implicit UAV
+// CHECK: -HLSLResourceAttr 0x{{[0-9a-f]+}} <<invalid sloc>> Implicit TypedBuffer
+RWBuffer<float> Buffer1;

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[damyanp]

I'm not sure if this is too remedial, but for someone with my limited experience a comment along the lines of "If this variable's type is actually an HLSL resource then annotate it appropriately" would have helped me greatly in understanding it.

[bogner]

Looks good modulo a couple of nitpicks.

[bogner]

With the change to use ArrayRef above, I find constructing the array as part of the call slightly more readable:

    Attr *ResourceClassAttr =
        HLSLResourceClassAttr::CreateImplicit(Record->getASTContext(), RC);
    Attr *ResourceAttr =
        HLSLResourceClassAttr::CreateImplicit(Record->getASTContext(), RC);
    addMemberVariable("h", Ty, {ResourceClassAttr, ResourceAttr}, Access);