Skip to content

Conversation

hekota
Copy link
Member

@hekota hekota commented Aug 7, 2025

If a resource array does not have an explicit binding attribute, SemaHLSL will add an implicit one. The attribute will be used to transfer implicit binding order ID to the codegen, the same way as it is done for HLSLBufferDecls. This is necessary in order to generate correct initialization of resources in an array that does not have an explicit binding.

Depends on #152450

Part 1 of #145424

hekota added 3 commits August 7, 2025 00:03
…ing and make them static

If a resource array does not have an explicit binding attribute, SemaHLSL will add
an implicit one. The attribute will be used to transfer implicit binding order ID
to the codegen, the same way as it is done for HLSLBufferDecls. This is necessary
in order to generate correct initialization of resources in an array that does not
have an explicit binding.

This change also marks resource arrays declared at a global scope as `static`, which
is what is already done for standalone resources.
@hekota hekota changed the title [HLSL] Add implicit binding attribute to resource arrays and make them static [HLSL] Add implicit binding attribute to resource arrays Aug 7, 2025
@hekota hekota marked this pull request as ready for review August 7, 2025 15:37
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" HLSL HLSL Language Support labels Aug 7, 2025
@llvmbot
Copy link
Member

llvmbot commented Aug 7, 2025

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-hlsl

Author: Helena Kotas (hekota)

Changes

If a resource array does not have an explicit binding attribute, SemaHLSL will add an implicit one. The attribute will be used to transfer implicit binding order ID to the codegen, the same way as it is done for HLSLBufferDecls. This is necessary in order to generate correct initialization of resources in an array that does not have an explicit binding.

Depends on #152450

Part 1 of #145424


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

2 Files Affected:

  • (modified) clang/lib/Sema/SemaHLSL.cpp (+44-11)
  • (modified) clang/test/AST/HLSL/resource_binding_attr.hlsl (+20)
diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index 17f17f8114373..6811f3f27603b 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -71,6 +71,10 @@ static RegisterType getRegisterType(ResourceClass RC) {
   llvm_unreachable("unexpected ResourceClass value");
 }
 
+static RegisterType getRegisterType(const HLSLAttributedResourceType *ResTy) {
+  return getRegisterType(ResTy->getAttrs().ResourceClass);
+}
+
 // Converts the first letter of string Slot to RegisterType.
 // Returns false if the letter does not correspond to a valid register type.
 static bool convertToRegisterType(StringRef Slot, RegisterType *RT) {
@@ -342,6 +346,17 @@ static bool isResourceRecordTypeOrArrayOf(VarDecl *VD) {
   return Ty->isHLSLResourceRecord() || Ty->isHLSLResourceRecordArray();
 }
 
+static const HLSLAttributedResourceType *
+getResourceArrayHandleType(VarDecl *VD) {
+  assert(VD->getType()->isHLSLResourceRecordArray() &&
+         "expected array of resource records");
+  const Type *Ty = VD->getType()->getUnqualifiedDesugaredType();
+  while (const ConstantArrayType *CAT = dyn_cast<ConstantArrayType>(Ty)) {
+    Ty = CAT->getArrayElementTypeNoTypeQual()->getUnqualifiedDesugaredType();
+  }
+  return HLSLAttributedResourceType::findHandleTypeOnResource(Ty);
+}
+
 // Returns true if the type is a leaf element type that is not valid to be
 // included in HLSL Buffer, such as a resource class, empty struct, zero-sized
 // array, or a builtin intangible type. Returns false it is a valid leaf element
@@ -568,16 +583,13 @@ void createHostLayoutStructForBuffer(Sema &S, HLSLBufferDecl *BufDecl) {
   BufDecl->addLayoutStruct(LS);
 }
 
-static void addImplicitBindingAttrToBuffer(Sema &S, HLSLBufferDecl *BufDecl,
-                                           uint32_t ImplicitBindingOrderID) {
-  RegisterType RT =
-      BufDecl->isCBuffer() ? RegisterType::CBuffer : RegisterType::SRV;
+static void addImplicitBindingAttrToDecl(Sema &S, Decl *D, RegisterType RT,
+                                         uint32_t ImplicitBindingOrderID) {
   auto *Attr =
       HLSLResourceBindingAttr::CreateImplicit(S.getASTContext(), "", "0", {});
-  std::optional<unsigned> RegSlot;
-  Attr->setBinding(RT, RegSlot, 0);
+  Attr->setBinding(RT, std::nullopt, 0);
   Attr->setImplicitBindingOrderID(ImplicitBindingOrderID);
-  BufDecl->addAttr(Attr);
+  D->addAttr(Attr);
 }
 
 // Handle end of cbuffer/tbuffer declaration
@@ -600,7 +612,10 @@ void SemaHLSL::ActOnFinishBuffer(Decl *Dcl, SourceLocation RBrace) {
     if (RBA)
       RBA->setImplicitBindingOrderID(OrderID);
     else
-      addImplicitBindingAttrToBuffer(SemaRef, BufDecl, OrderID);
+      addImplicitBindingAttrToDecl(SemaRef, BufDecl,
+                                   BufDecl->isCBuffer() ? RegisterType::CBuffer
+                                                        : RegisterType::SRV,
+                                   OrderID);
   }
 
   SemaRef.PopDeclContext();
@@ -1906,7 +1921,7 @@ static bool DiagnoseLocalRegisterBinding(Sema &S, SourceLocation &ArgLoc,
   if (const HLSLAttributedResourceType *AttrResType =
           HLSLAttributedResourceType::findHandleTypeOnResource(
               VD->getType().getTypePtr())) {
-    if (RegType == getRegisterType(AttrResType->getAttrs().ResourceClass))
+    if (RegType == getRegisterType(AttrResType))
       return true;
 
     S.Diag(D->getLocation(), diag::err_hlsl_binding_type_mismatch)
@@ -2439,8 +2454,8 @@ void SemaHLSL::ActOnEndOfTranslationUnit(TranslationUnitDecl *TU) {
     HLSLBufferDecl *DefaultCBuffer = HLSLBufferDecl::CreateDefaultCBuffer(
         SemaRef.getASTContext(), SemaRef.getCurLexicalContext(),
         DefaultCBufferDecls);
-    addImplicitBindingAttrToBuffer(SemaRef, DefaultCBuffer,
-                                   getNextImplicitBindingOrderID());
+    addImplicitBindingAttrToDecl(SemaRef, DefaultCBuffer, RegisterType::CBuffer,
+                                 getNextImplicitBindingOrderID());
     SemaRef.getCurLexicalContext()->addDecl(DefaultCBuffer);
     createHostLayoutStructForBuffer(SemaRef, DefaultCBuffer);
 
@@ -3640,6 +3655,24 @@ void SemaHLSL::ActOnVariableDeclarator(VarDecl *VD) {
 
     // process explicit bindings
     processExplicitBindingsOnDecl(VD);
+
+    if (VD->getType()->isHLSLResourceRecordArray()) {
+      // If the resource array does not have an explicit binding attribute,
+      // create an implicit one. It will be used to transfer implicit binding
+      // order_ID to codegen.
+      if (!VD->hasAttr<HLSLVkBindingAttr>()) {
+        HLSLResourceBindingAttr *RBA = VD->getAttr<HLSLResourceBindingAttr>();
+        if (!RBA || !RBA->hasRegisterSlot()) {
+          uint32_t OrderID = getNextImplicitBindingOrderID();
+          if (RBA)
+            RBA->setImplicitBindingOrderID(OrderID);
+          else
+            addImplicitBindingAttrToDecl(
+                SemaRef, VD, getRegisterType(getResourceArrayHandleType(VD)),
+                OrderID);
+        }
+      }
+    }
   }
 
   deduceAddressSpace(VD);
diff --git a/clang/test/AST/HLSL/resource_binding_attr.hlsl b/clang/test/AST/HLSL/resource_binding_attr.hlsl
index c073cd4dc1476..05d5eb0d619d8 100644
--- a/clang/test/AST/HLSL/resource_binding_attr.hlsl
+++ b/clang/test/AST/HLSL/resource_binding_attr.hlsl
@@ -34,6 +34,10 @@ RWBuffer<float> UAV1 : register(u2), UAV2 : register(u4);
 // CHECK: HLSLResourceBindingAttr {{.*}} "" "space5"
 RWBuffer<float> UAV3 : register(space5);
 
+// CHECK: VarDecl {{.*}} UAV_Array 'RWBuffer<float>[10]'
+// CHECK: HLSLResourceBindingAttr {{.*}} "u10" "space6"
+RWBuffer<float> UAV_Array[10] : register(u10, space6);
+
 //
 // Default constants ($Globals) layout annotations
 
@@ -56,3 +60,19 @@ struct S {
 // CHECK: VarDecl {{.*}} s 'hlsl_constant S'
 // CHECK: HLSLResourceBindingAttr {{.*}} "c10" "space0
 S s : register(c10);
+
+//
+// Implicit binding
+
+// Constant buffers should have implicit binding attribute added by SemaHLSL
+// CHECK: HLSLBufferDecl {{.*}} line:[[# @LINE + 3]]:9 cbuffer CB2
+// CHECK-NEXT: HLSLResourceClassAttr {{.*}} Implicit CBuffer
+// CHECK-NEXT: HLSLResourceBindingAttr {{.*}} Implicit "" "0"
+cbuffer CB2 {
+  float4 c;
+}
+
+// Resource arrays should have implicit binding attribute added by SemaHLSL
+// CHECK: VarDecl {{.*}} SB 'StructuredBuffer<float>[10]'
+// CHECK: HLSLResourceBindingAttr {{.*}} Implicit "" "0"
+StructuredBuffer<float> SB[10];

@hekota hekota changed the base branch from users/hekota/pr152450-add-is-record-array-to-type to main August 7, 2025 21:33
@hekota hekota merged commit 662636e into llvm:main Aug 11, 2025
10 of 11 checks passed
hekota added a commit that referenced this pull request Aug 19, 2025
Adds support for accessing individual resources from fixed-size global resource arrays.

Design proposal:
https://github.com/llvm/wg-hlsl/blob/main/proposals/0028-resource-arrays.md

Enables indexing into globally scoped, fixed-size resource arrays to retrieve individual resources. The initialization logic is primarily handled during codegen. When a global resource array is indexed, the
codegen translates the `ArraySubscriptExpr` AST node into a constructor call for the corresponding resource record type and binding.

To support this behavior, Sema needs to ensure that:
- The constructor for the specific resource type is instantiated.
- An implicit binding attribute is added to resource arrays that lack explicit bindings (#152452).

Closes #145424
@hekota hekota deleted the resource-array-impl-binding-attr branch August 19, 2025 16:58
Copy link
Collaborator

@shafik shafik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies for the post review but I found a bug.

HLSLResourceBindingAttr *RBA = VD->getAttr<HLSLResourceBindingAttr>();
if (!RBA || !RBA->hasRegisterSlot()) {
uint32_t OrderID = getNextImplicitBindingOrderID();
if (RBA)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't RBA have to be non-nullptr here?

Copy link
Member Author

@hekota hekota Aug 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually no. The condition !RBA || !RBA->hasRegisterSlot() is true when we either don't have an RBA, or when we have an RBA but without a register slot. In both cases we need to assign implicit binding, but in case that we don't have an RBA we need to create one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category HLSL HLSL Language Support

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants