Skip to content

Conversation

hekota
Copy link
Member

@hekota hekota commented Sep 29, 2025

Add new ResourceBindingAttrs struct that holds resource binding attributes HLSLResourceBindingAttr and HLSLVkBindingAttr and provides helper methods to simplify dealing with resource bindings. This code is placed in the AST library to be shared between Sema and CodeGen.

This change has been done in preparation of a third binding attribute coming soon to represent [[vk::counter_binding()]]. This new attribute and more helper member functions will be added to ResourceBindingAttrs and will be used in both Sema and in CodeGen to implement resource counter initialization.

…e binding attributes

Add new `ResourceBindingAttrs` struct that holds resource binding attributes HLSLResourceBindingAttr
and HLSLVkBindingAttr and provides helper member functions to simplify dealing with resource bindings.
This code is placed in the AST library because it is needed by both Sema and CodeGen.

This change has been done in preparation of a third binding attribute coming soon to represent `[[vk::counter_binding()]]`. This new attribute and more helper member functions will be added
to `ResourceBindingAttrs` and used where necessary in Sema and CodeGen.
@hekota hekota requested a review from s-perron September 29, 2025 18:19
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:codegen IR generation bugs: mangling, exceptions, etc. HLSL HLSL Language Support labels Sep 29, 2025
@llvmbot
Copy link
Member

llvmbot commented Sep 29, 2025

@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-hlsl

Author: Helena Kotas (hekota)

Changes

Add new ResourceBindingAttrs struct that holds resource binding attributes HLSLResourceBindingAttr and HLSLVkBindingAttr and provides helper methods to simplify dealing with resource bindings. This code is placed in the AST library to be shared between Sema and CodeGen.

This change has been done in preparation of a third binding attribute coming soon to represent [[vk::counter_binding()]]. This new attribute and more helper member functions will be added to ResourceBindingAttrs and will be used in both Sema and in CodeGen to implement resource counter initialization.


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

4 Files Affected:

  • (added) clang/include/clang/AST/HLSLResource.h (+75)
  • (modified) clang/lib/CodeGen/CGHLSLRuntime.cpp (+31-63)
  • (modified) clang/lib/CodeGen/CGHLSLRuntime.h (+1-5)
  • (modified) clang/lib/Sema/SemaHLSL.cpp (+11-20)
diff --git a/clang/include/clang/AST/HLSLResource.h b/clang/include/clang/AST/HLSLResource.h
new file mode 100644
index 0000000000000..e3ee0b136cec3
--- /dev/null
+++ b/clang/include/clang/AST/HLSLResource.h
@@ -0,0 +1,75 @@
+//===- HLSLResource.h - Routines for HLSL resources and bindings ----------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file provides shared routines to help analyze HLSL resources and
+// theirs bindings during Sema and CodeGen.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_AST_HLSLRESOURCE_H
+#define LLVM_CLANG_AST_HLSLRESOURCE_H
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Attrs.inc"
+#include "clang/AST/DeclBase.h"
+#include "clang/Basic/TargetInfo.h"
+
+namespace clang {
+
+class HLSLResourceBindingAttr;
+class HLSLRVkBindingAttr;
+
+namespace hlsl {
+
+struct ResourceBindingAttrs {
+  HLSLResourceBindingAttr *RegBinding;
+  HLSLVkBindingAttr *VkBinding;
+
+  ResourceBindingAttrs(const Decl *D) {
+    RegBinding = D->getAttr<HLSLResourceBindingAttr>();
+    bool IsSpirv = D->getASTContext().getTargetInfo().getTriple().isSPIRV();
+    VkBinding = IsSpirv ? D->getAttr<HLSLVkBindingAttr>() : nullptr;
+  }
+
+  bool hasBinding() const { return RegBinding || VkBinding; }
+  bool isExplicit() const {
+    return (RegBinding && RegBinding->hasRegisterSlot()) || VkBinding;
+  }
+
+  unsigned getSlot() const {
+    assert(isExplicit() && "no explicit binding");
+    if (VkBinding)
+      return VkBinding->getBinding();
+    if (RegBinding && RegBinding->hasRegisterSlot())
+      return RegBinding->getSlotNumber();
+    llvm_unreachable("no explicit binding");
+  }
+
+  unsigned getSpace() const {
+    if (VkBinding)
+      return VkBinding->getSet();
+    if (RegBinding)
+      return RegBinding->getSpaceNumber();
+    return 0;
+  }
+
+  bool hasImplicitOrderID() const {
+    return RegBinding && RegBinding->hasImplicitBindingOrderID();
+  }
+
+  unsigned getImplicitOrderID() const {
+    assert(hasImplicitOrderID());
+    return RegBinding->getImplicitBindingOrderID();
+  }
+};
+
+} // namespace hlsl
+
+} // namespace clang
+
+#endif // LLVM_CLANG_AST_HLSLRESOURCE_H
diff --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp b/clang/lib/CodeGen/CGHLSLRuntime.cpp
index cf018c8c7de2a..ebe6d605812fd 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -22,6 +22,7 @@
 #include "clang/AST/Attrs.inc"
 #include "clang/AST/Decl.h"
 #include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/HLSLResource.h"
 #include "clang/AST/Type.h"
 #include "clang/Basic/TargetOptions.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
@@ -131,35 +132,24 @@ static CXXMethodDecl *lookupMethod(CXXRecordDecl *Record, StringRef Name,
 
 static CXXMethodDecl *lookupResourceInitMethodAndSetupArgs(
     CodeGenModule &CGM, CXXRecordDecl *ResourceDecl, llvm::Value *Range,
-    llvm::Value *Index, StringRef Name, HLSLResourceBindingAttr *RBA,
-    HLSLVkBindingAttr *VkBinding, CallArgList &Args) {
-  assert((VkBinding || RBA) && "at least one a binding attribute expected");
+    llvm::Value *Index, StringRef Name, ResourceBindingAttrs &Binding,
+    CallArgList &Args) {
+  assert(Binding.hasBinding() && "at least one binding attribute expected");
 
   ASTContext &AST = CGM.getContext();
-  std::optional<uint32_t> RegisterSlot;
-  uint32_t SpaceNo = 0;
-  if (VkBinding) {
-    RegisterSlot = VkBinding->getBinding();
-    SpaceNo = VkBinding->getSet();
-  } else {
-    if (RBA->hasRegisterSlot())
-      RegisterSlot = RBA->getSlotNumber();
-    SpaceNo = RBA->getSpaceNumber();
-  }
-
   CXXMethodDecl *CreateMethod = nullptr;
   Value *NameStr = buildNameForResource(Name, CGM);
-  Value *Space = llvm::ConstantInt::get(CGM.IntTy, SpaceNo);
+  Value *Space = llvm::ConstantInt::get(CGM.IntTy, Binding.getSpace());
 
-  if (RegisterSlot.has_value()) {
+  if (Binding.isExplicit()) {
     // explicit binding
-    auto *RegSlot = llvm::ConstantInt::get(CGM.IntTy, RegisterSlot.value());
+    auto *RegSlot = llvm::ConstantInt::get(CGM.IntTy, Binding.getSlot());
     Args.add(RValue::get(RegSlot), AST.UnsignedIntTy);
     CreateMethod = lookupMethod(ResourceDecl, "__createFromBinding", SC_Static);
   } else {
     // implicit binding
     auto *OrderID =
-        llvm::ConstantInt::get(CGM.IntTy, RBA->getImplicitBindingOrderID());
+        llvm::ConstantInt::get(CGM.IntTy, Binding.getImplicitOrderID());
     Args.add(RValue::get(OrderID), AST.UnsignedIntTy);
     CreateMethod =
         lookupMethod(ResourceDecl, "__createFromImplicitBinding", SC_Static);
@@ -194,8 +184,8 @@ static std::optional<llvm::Value *> initializeLocalResourceArray(
     CodeGenFunction &CGF, CXXRecordDecl *ResourceDecl,
     const ConstantArrayType *ArrayTy, AggValueSlot &ValueSlot,
     llvm::Value *Range, llvm::Value *StartIndex, StringRef ResourceName,
-    HLSLResourceBindingAttr *RBA, HLSLVkBindingAttr *VkBinding,
-    ArrayRef<llvm::Value *> PrevGEPIndices, SourceLocation ArraySubsExprLoc) {
+    ResourceBindingAttrs &Binding, ArrayRef<llvm::Value *> PrevGEPIndices,
+    SourceLocation ArraySubsExprLoc) {
 
   ASTContext &AST = CGF.getContext();
   llvm::IntegerType *IntTy = CGF.CGM.IntTy;
@@ -220,7 +210,7 @@ static std::optional<llvm::Value *> initializeLocalResourceArray(
       }
       std::optional<llvm::Value *> MaybeIndex = initializeLocalResourceArray(
           CGF, ResourceDecl, SubArrayTy, ValueSlot, Range, Index, ResourceName,
-          RBA, VkBinding, GEPIndices, ArraySubsExprLoc);
+          Binding, GEPIndices, ArraySubsExprLoc);
       if (!MaybeIndex)
         return std::nullopt;
       Index = *MaybeIndex;
@@ -244,7 +234,7 @@ static std::optional<llvm::Value *> initializeLocalResourceArray(
 
     CallArgList Args;
     CXXMethodDecl *CreateMethod = lookupResourceInitMethodAndSetupArgs(
-        CGF.CGM, ResourceDecl, Range, Index, ResourceName, RBA, VkBinding,
+        CGF.CGM, ResourceDecl, Range, Index, ResourceName, Binding,
         Args);
 
     if (!CreateMethod)
@@ -439,14 +429,7 @@ void CGHLSLRuntime::addBuffer(const HLSLBufferDecl *BufDecl) {
   emitBufferGlobalsAndMetadata(BufDecl, BufGV);
 
   // Initialize cbuffer from binding (implicit or explicit)
-  if (HLSLVkBindingAttr *VkBinding = BufDecl->getAttr<HLSLVkBindingAttr>()) {
-    initializeBufferFromBinding(BufDecl, BufGV, VkBinding);
-  } else {
-    HLSLResourceBindingAttr *RBA = BufDecl->getAttr<HLSLResourceBindingAttr>();
-    assert(RBA &&
-           "cbuffer/tbuffer should always have resource binding attribute");
-    initializeBufferFromBinding(BufDecl, BufGV, RBA);
-  }
+  initializeBufferFromBinding(BufDecl, BufGV);
 }
 
 void CGHLSLRuntime::addRootSignature(
@@ -810,44 +793,29 @@ static void initializeBuffer(CodeGenModule &CGM, llvm::GlobalVariable *GV,
 }
 
 void CGHLSLRuntime::initializeBufferFromBinding(const HLSLBufferDecl *BufDecl,
-                                                llvm::GlobalVariable *GV,
-                                                HLSLVkBindingAttr *VkBinding) {
-  assert(VkBinding && "expect a nonnull binding attribute");
-  auto *Index = llvm::ConstantInt::get(CGM.IntTy, 0);
-  auto *RangeSize = llvm::ConstantInt::get(CGM.IntTy, 1);
-  auto *Set = llvm::ConstantInt::get(CGM.IntTy, VkBinding->getSet());
-  auto *Binding = llvm::ConstantInt::get(CGM.IntTy, VkBinding->getBinding());
-  Value *Name = buildNameForResource(BufDecl->getName(), CGM);
-  llvm::Intrinsic::ID IntrinsicID =
-      CGM.getHLSLRuntime().getCreateHandleFromBindingIntrinsic();
-
-  SmallVector<Value *> Args{Set, Binding, RangeSize, Index, Name};
-  initializeBuffer(CGM, GV, IntrinsicID, Args);
-}
+                                                llvm::GlobalVariable *GV) {
+  ResourceBindingAttrs Binding(BufDecl);
+  assert(Binding.hasBinding() &&
+         "cbuffer/tbuffer should always have resource binding attribute");
 
-void CGHLSLRuntime::initializeBufferFromBinding(const HLSLBufferDecl *BufDecl,
-                                                llvm::GlobalVariable *GV,
-                                                HLSLResourceBindingAttr *RBA) {
-  assert(RBA && "expect a nonnull binding attribute");
   auto *Index = llvm::ConstantInt::get(CGM.IntTy, 0);
   auto *RangeSize = llvm::ConstantInt::get(CGM.IntTy, 1);
-  auto *Space = llvm::ConstantInt::get(CGM.IntTy, RBA->getSpaceNumber());
+  auto *Space = llvm::ConstantInt::get(CGM.IntTy, Binding.getSpace());
   Value *Name = buildNameForResource(BufDecl->getName(), CGM);
 
-  llvm::Intrinsic::ID IntrinsicID =
-      RBA->hasRegisterSlot()
-          ? CGM.getHLSLRuntime().getCreateHandleFromBindingIntrinsic()
-          : CGM.getHLSLRuntime().getCreateHandleFromImplicitBindingIntrinsic();
-
   // buffer with explicit binding
-  if (RBA->hasRegisterSlot()) {
-    auto *RegSlot = llvm::ConstantInt::get(CGM.IntTy, RBA->getSlotNumber());
+  if (Binding.isExplicit()) {
+    llvm::Intrinsic::ID IntrinsicID =
+        CGM.getHLSLRuntime().getCreateHandleFromBindingIntrinsic();
+    auto *RegSlot = llvm::ConstantInt::get(CGM.IntTy, Binding.getSlot());
     SmallVector<Value *> Args{Space, RegSlot, RangeSize, Index, Name};
     initializeBuffer(CGM, GV, IntrinsicID, Args);
   } else {
     // buffer with implicit binding
+    llvm::Intrinsic::ID IntrinsicID =
+        CGM.getHLSLRuntime().getCreateHandleFromImplicitBindingIntrinsic();
     auto *OrderID =
-        llvm::ConstantInt::get(CGM.IntTy, RBA->getImplicitBindingOrderID());
+        llvm::ConstantInt::get(CGM.IntTy, Binding.getImplicitOrderID());
     SmallVector<Value *> Args{OrderID, Space, RangeSize, Index, Name};
     initializeBuffer(CGM, GV, IntrinsicID, Args);
   }
@@ -960,9 +928,9 @@ std::optional<LValue> CGHLSLRuntime::emitResourceArraySubscriptExpr(
 
   // Find binding info for the resource array. For implicit binding
   // an HLSLResourceBindingAttr should have been added by SemaHLSL.
-  HLSLVkBindingAttr *VkBinding = ArrayDecl->getAttr<HLSLVkBindingAttr>();
-  HLSLResourceBindingAttr *RBA = ArrayDecl->getAttr<HLSLResourceBindingAttr>();
-  assert((VkBinding || RBA) && "resource array must have a binding attribute");
+  ResourceBindingAttrs Binding(ArrayDecl);
+  assert((Binding.hasBinding()) &&
+         "resource array must have a binding attribute");
 
   // Find the individual resource type.
   QualType ResultTy = ArraySubsExpr->getType();
@@ -992,7 +960,7 @@ std::optional<LValue> CGHLSLRuntime::emitResourceArraySubscriptExpr(
     CallArgList Args;
     CXXMethodDecl *CreateMethod = lookupResourceInitMethodAndSetupArgs(
         CGF.CGM, ResourceTy->getAsCXXRecordDecl(), Range, Index,
-        ArrayDecl->getName(), RBA, VkBinding, Args);
+        ArrayDecl->getName(), Binding, Args);
 
     if (!CreateMethod)
       // This can happen if someone creates an array of structs that looks like
@@ -1009,8 +977,8 @@ std::optional<LValue> CGHLSLRuntime::emitResourceArraySubscriptExpr(
         cast<ConstantArrayType>(ResultTy.getTypePtr());
     std::optional<llvm::Value *> EndIndex = initializeLocalResourceArray(
         CGF, ResourceTy->getAsCXXRecordDecl(), ArrayTy, ValueSlot, Range, Index,
-        ArrayDecl->getName(), RBA, VkBinding,
-        {llvm::ConstantInt::get(CGM.IntTy, 0)}, ArraySubsExpr->getExprLoc());
+        ArrayDecl->getName(), Binding, {llvm::ConstantInt::get(CGM.IntTy, 0)},
+        ArraySubsExpr->getExprLoc());
     if (!EndIndex)
       return std::nullopt;
   }
diff --git a/clang/lib/CodeGen/CGHLSLRuntime.h b/clang/lib/CodeGen/CGHLSLRuntime.h
index 9c0e6056fd4ee..7c6c2850fd4d4 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.h
+++ b/clang/lib/CodeGen/CGHLSLRuntime.h
@@ -200,11 +200,7 @@ class CGHLSLRuntime {
   void emitBufferGlobalsAndMetadata(const HLSLBufferDecl *BufDecl,
                                     llvm::GlobalVariable *BufGV);
   void initializeBufferFromBinding(const HLSLBufferDecl *BufDecl,
-                                   llvm::GlobalVariable *GV,
-                                   HLSLVkBindingAttr *VkBinding);
-  void initializeBufferFromBinding(const HLSLBufferDecl *BufDecl,
-                                   llvm::GlobalVariable *GV,
-                                   HLSLResourceBindingAttr *RBA);
+                                   llvm::GlobalVariable *GV);
   llvm::Triple::ArchType getArch();
 
   llvm::DenseMap<const clang::RecordType *, llvm::TargetExtType *> LayoutTypes;
diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index 940d510b4cc02..129b03c07c0bd 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -19,6 +19,7 @@
 #include "clang/AST/DeclarationName.h"
 #include "clang/AST/DynamicRecursiveASTVisitor.h"
 #include "clang/AST/Expr.h"
+#include "clang/AST/HLSLResource.h"
 #include "clang/AST/Type.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/Builtins.h"
@@ -52,6 +53,7 @@
 #include <utility>
 
 using namespace clang;
+using namespace clang::hlsl;
 using RegisterType = HLSLResourceBindingAttr::RegisterType;
 
 static CXXRecordDecl *createHostLayoutStruct(Sema &S,
@@ -3799,19 +3801,8 @@ bool SemaHLSL::initGlobalResourceDecl(VarDecl *VD) {
   uint64_t UIntTySize = AST.getTypeSize(AST.UnsignedIntTy);
   uint64_t IntTySize = AST.getTypeSize(AST.IntTy);
 
-  // Gather resource binding information from attributes.
-  HLSLResourceBindingAttr *RBA = VD->getAttr<HLSLResourceBindingAttr>();
-  HLSLVkBindingAttr *VkBinding = VD->getAttr<HLSLVkBindingAttr>();
-  std::optional<uint32_t> RegisterSlot;
-  uint32_t SpaceNo = 0;
-  if (VkBinding) {
-    RegisterSlot = VkBinding->getBinding();
-    SpaceNo = VkBinding->getSet();
-  } else if (RBA) {
-    if (RBA->hasRegisterSlot())
-      RegisterSlot = RBA->getSlotNumber();
-    SpaceNo = RBA->getSpaceNumber();
-  }
+  // Gather resource binding attributes.
+  ResourceBindingAttrs Binding(VD);
 
   // Find correct initialization method and create its arguments.
   QualType ResourceTy = VD->getType();
@@ -3819,21 +3810,21 @@ bool SemaHLSL::initGlobalResourceDecl(VarDecl *VD) {
   CXXMethodDecl *CreateMethod = nullptr;
   llvm::SmallVector<Expr *> Args;
 
-  if (RegisterSlot.has_value()) {
+  if (Binding.isExplicit()) {
     // The resource has explicit binding.
     CreateMethod = lookupMethod(SemaRef, ResourceDecl, "__createFromBinding",
                                 VD->getLocation());
-    IntegerLiteral *RegSlot = IntegerLiteral::Create(
-        AST, llvm::APInt(UIntTySize, RegisterSlot.value()), AST.UnsignedIntTy,
-        SourceLocation());
+    IntegerLiteral *RegSlot =
+        IntegerLiteral::Create(AST, llvm::APInt(UIntTySize, Binding.getSlot()),
+                               AST.UnsignedIntTy, SourceLocation());
     Args.push_back(RegSlot);
   } else {
     // The resource has implicit binding.
     CreateMethod =
         lookupMethod(SemaRef, ResourceDecl, "__createFromImplicitBinding",
                      VD->getLocation());
-    uint32_t OrderID = (RBA && RBA->hasImplicitBindingOrderID())
-                           ? RBA->getImplicitBindingOrderID()
+    uint32_t OrderID = (Binding.hasImplicitOrderID())
+                           ? Binding.getImplicitOrderID()
                            : getNextImplicitBindingOrderID();
     IntegerLiteral *OrderId =
         IntegerLiteral::Create(AST, llvm::APInt(UIntTySize, OrderID),
@@ -3848,7 +3839,7 @@ bool SemaHLSL::initGlobalResourceDecl(VarDecl *VD) {
     return false;
 
   IntegerLiteral *Space =
-      IntegerLiteral::Create(AST, llvm::APInt(UIntTySize, SpaceNo),
+      IntegerLiteral::Create(AST, llvm::APInt(UIntTySize, Binding.getSpace()),
                              AST.UnsignedIntTy, SourceLocation());
   Args.push_back(Space);
 

@llvmbot
Copy link
Member

llvmbot commented Sep 29, 2025

@llvm/pr-subscribers-clang

Author: Helena Kotas (hekota)

Changes

Add new ResourceBindingAttrs struct that holds resource binding attributes HLSLResourceBindingAttr and HLSLVkBindingAttr and provides helper methods to simplify dealing with resource bindings. This code is placed in the AST library to be shared between Sema and CodeGen.

This change has been done in preparation of a third binding attribute coming soon to represent [[vk::counter_binding()]]. This new attribute and more helper member functions will be added to ResourceBindingAttrs and will be used in both Sema and in CodeGen to implement resource counter initialization.


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

4 Files Affected:

  • (added) clang/include/clang/AST/HLSLResource.h (+75)
  • (modified) clang/lib/CodeGen/CGHLSLRuntime.cpp (+31-63)
  • (modified) clang/lib/CodeGen/CGHLSLRuntime.h (+1-5)
  • (modified) clang/lib/Sema/SemaHLSL.cpp (+11-20)
diff --git a/clang/include/clang/AST/HLSLResource.h b/clang/include/clang/AST/HLSLResource.h
new file mode 100644
index 0000000000000..e3ee0b136cec3
--- /dev/null
+++ b/clang/include/clang/AST/HLSLResource.h
@@ -0,0 +1,75 @@
+//===- HLSLResource.h - Routines for HLSL resources and bindings ----------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file provides shared routines to help analyze HLSL resources and
+// theirs bindings during Sema and CodeGen.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_AST_HLSLRESOURCE_H
+#define LLVM_CLANG_AST_HLSLRESOURCE_H
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Attrs.inc"
+#include "clang/AST/DeclBase.h"
+#include "clang/Basic/TargetInfo.h"
+
+namespace clang {
+
+class HLSLResourceBindingAttr;
+class HLSLRVkBindingAttr;
+
+namespace hlsl {
+
+struct ResourceBindingAttrs {
+  HLSLResourceBindingAttr *RegBinding;
+  HLSLVkBindingAttr *VkBinding;
+
+  ResourceBindingAttrs(const Decl *D) {
+    RegBinding = D->getAttr<HLSLResourceBindingAttr>();
+    bool IsSpirv = D->getASTContext().getTargetInfo().getTriple().isSPIRV();
+    VkBinding = IsSpirv ? D->getAttr<HLSLVkBindingAttr>() : nullptr;
+  }
+
+  bool hasBinding() const { return RegBinding || VkBinding; }
+  bool isExplicit() const {
+    return (RegBinding && RegBinding->hasRegisterSlot()) || VkBinding;
+  }
+
+  unsigned getSlot() const {
+    assert(isExplicit() && "no explicit binding");
+    if (VkBinding)
+      return VkBinding->getBinding();
+    if (RegBinding && RegBinding->hasRegisterSlot())
+      return RegBinding->getSlotNumber();
+    llvm_unreachable("no explicit binding");
+  }
+
+  unsigned getSpace() const {
+    if (VkBinding)
+      return VkBinding->getSet();
+    if (RegBinding)
+      return RegBinding->getSpaceNumber();
+    return 0;
+  }
+
+  bool hasImplicitOrderID() const {
+    return RegBinding && RegBinding->hasImplicitBindingOrderID();
+  }
+
+  unsigned getImplicitOrderID() const {
+    assert(hasImplicitOrderID());
+    return RegBinding->getImplicitBindingOrderID();
+  }
+};
+
+} // namespace hlsl
+
+} // namespace clang
+
+#endif // LLVM_CLANG_AST_HLSLRESOURCE_H
diff --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp b/clang/lib/CodeGen/CGHLSLRuntime.cpp
index cf018c8c7de2a..ebe6d605812fd 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -22,6 +22,7 @@
 #include "clang/AST/Attrs.inc"
 #include "clang/AST/Decl.h"
 #include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/HLSLResource.h"
 #include "clang/AST/Type.h"
 #include "clang/Basic/TargetOptions.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
@@ -131,35 +132,24 @@ static CXXMethodDecl *lookupMethod(CXXRecordDecl *Record, StringRef Name,
 
 static CXXMethodDecl *lookupResourceInitMethodAndSetupArgs(
     CodeGenModule &CGM, CXXRecordDecl *ResourceDecl, llvm::Value *Range,
-    llvm::Value *Index, StringRef Name, HLSLResourceBindingAttr *RBA,
-    HLSLVkBindingAttr *VkBinding, CallArgList &Args) {
-  assert((VkBinding || RBA) && "at least one a binding attribute expected");
+    llvm::Value *Index, StringRef Name, ResourceBindingAttrs &Binding,
+    CallArgList &Args) {
+  assert(Binding.hasBinding() && "at least one binding attribute expected");
 
   ASTContext &AST = CGM.getContext();
-  std::optional<uint32_t> RegisterSlot;
-  uint32_t SpaceNo = 0;
-  if (VkBinding) {
-    RegisterSlot = VkBinding->getBinding();
-    SpaceNo = VkBinding->getSet();
-  } else {
-    if (RBA->hasRegisterSlot())
-      RegisterSlot = RBA->getSlotNumber();
-    SpaceNo = RBA->getSpaceNumber();
-  }
-
   CXXMethodDecl *CreateMethod = nullptr;
   Value *NameStr = buildNameForResource(Name, CGM);
-  Value *Space = llvm::ConstantInt::get(CGM.IntTy, SpaceNo);
+  Value *Space = llvm::ConstantInt::get(CGM.IntTy, Binding.getSpace());
 
-  if (RegisterSlot.has_value()) {
+  if (Binding.isExplicit()) {
     // explicit binding
-    auto *RegSlot = llvm::ConstantInt::get(CGM.IntTy, RegisterSlot.value());
+    auto *RegSlot = llvm::ConstantInt::get(CGM.IntTy, Binding.getSlot());
     Args.add(RValue::get(RegSlot), AST.UnsignedIntTy);
     CreateMethod = lookupMethod(ResourceDecl, "__createFromBinding", SC_Static);
   } else {
     // implicit binding
     auto *OrderID =
-        llvm::ConstantInt::get(CGM.IntTy, RBA->getImplicitBindingOrderID());
+        llvm::ConstantInt::get(CGM.IntTy, Binding.getImplicitOrderID());
     Args.add(RValue::get(OrderID), AST.UnsignedIntTy);
     CreateMethod =
         lookupMethod(ResourceDecl, "__createFromImplicitBinding", SC_Static);
@@ -194,8 +184,8 @@ static std::optional<llvm::Value *> initializeLocalResourceArray(
     CodeGenFunction &CGF, CXXRecordDecl *ResourceDecl,
     const ConstantArrayType *ArrayTy, AggValueSlot &ValueSlot,
     llvm::Value *Range, llvm::Value *StartIndex, StringRef ResourceName,
-    HLSLResourceBindingAttr *RBA, HLSLVkBindingAttr *VkBinding,
-    ArrayRef<llvm::Value *> PrevGEPIndices, SourceLocation ArraySubsExprLoc) {
+    ResourceBindingAttrs &Binding, ArrayRef<llvm::Value *> PrevGEPIndices,
+    SourceLocation ArraySubsExprLoc) {
 
   ASTContext &AST = CGF.getContext();
   llvm::IntegerType *IntTy = CGF.CGM.IntTy;
@@ -220,7 +210,7 @@ static std::optional<llvm::Value *> initializeLocalResourceArray(
       }
       std::optional<llvm::Value *> MaybeIndex = initializeLocalResourceArray(
           CGF, ResourceDecl, SubArrayTy, ValueSlot, Range, Index, ResourceName,
-          RBA, VkBinding, GEPIndices, ArraySubsExprLoc);
+          Binding, GEPIndices, ArraySubsExprLoc);
       if (!MaybeIndex)
         return std::nullopt;
       Index = *MaybeIndex;
@@ -244,7 +234,7 @@ static std::optional<llvm::Value *> initializeLocalResourceArray(
 
     CallArgList Args;
     CXXMethodDecl *CreateMethod = lookupResourceInitMethodAndSetupArgs(
-        CGF.CGM, ResourceDecl, Range, Index, ResourceName, RBA, VkBinding,
+        CGF.CGM, ResourceDecl, Range, Index, ResourceName, Binding,
         Args);
 
     if (!CreateMethod)
@@ -439,14 +429,7 @@ void CGHLSLRuntime::addBuffer(const HLSLBufferDecl *BufDecl) {
   emitBufferGlobalsAndMetadata(BufDecl, BufGV);
 
   // Initialize cbuffer from binding (implicit or explicit)
-  if (HLSLVkBindingAttr *VkBinding = BufDecl->getAttr<HLSLVkBindingAttr>()) {
-    initializeBufferFromBinding(BufDecl, BufGV, VkBinding);
-  } else {
-    HLSLResourceBindingAttr *RBA = BufDecl->getAttr<HLSLResourceBindingAttr>();
-    assert(RBA &&
-           "cbuffer/tbuffer should always have resource binding attribute");
-    initializeBufferFromBinding(BufDecl, BufGV, RBA);
-  }
+  initializeBufferFromBinding(BufDecl, BufGV);
 }
 
 void CGHLSLRuntime::addRootSignature(
@@ -810,44 +793,29 @@ static void initializeBuffer(CodeGenModule &CGM, llvm::GlobalVariable *GV,
 }
 
 void CGHLSLRuntime::initializeBufferFromBinding(const HLSLBufferDecl *BufDecl,
-                                                llvm::GlobalVariable *GV,
-                                                HLSLVkBindingAttr *VkBinding) {
-  assert(VkBinding && "expect a nonnull binding attribute");
-  auto *Index = llvm::ConstantInt::get(CGM.IntTy, 0);
-  auto *RangeSize = llvm::ConstantInt::get(CGM.IntTy, 1);
-  auto *Set = llvm::ConstantInt::get(CGM.IntTy, VkBinding->getSet());
-  auto *Binding = llvm::ConstantInt::get(CGM.IntTy, VkBinding->getBinding());
-  Value *Name = buildNameForResource(BufDecl->getName(), CGM);
-  llvm::Intrinsic::ID IntrinsicID =
-      CGM.getHLSLRuntime().getCreateHandleFromBindingIntrinsic();
-
-  SmallVector<Value *> Args{Set, Binding, RangeSize, Index, Name};
-  initializeBuffer(CGM, GV, IntrinsicID, Args);
-}
+                                                llvm::GlobalVariable *GV) {
+  ResourceBindingAttrs Binding(BufDecl);
+  assert(Binding.hasBinding() &&
+         "cbuffer/tbuffer should always have resource binding attribute");
 
-void CGHLSLRuntime::initializeBufferFromBinding(const HLSLBufferDecl *BufDecl,
-                                                llvm::GlobalVariable *GV,
-                                                HLSLResourceBindingAttr *RBA) {
-  assert(RBA && "expect a nonnull binding attribute");
   auto *Index = llvm::ConstantInt::get(CGM.IntTy, 0);
   auto *RangeSize = llvm::ConstantInt::get(CGM.IntTy, 1);
-  auto *Space = llvm::ConstantInt::get(CGM.IntTy, RBA->getSpaceNumber());
+  auto *Space = llvm::ConstantInt::get(CGM.IntTy, Binding.getSpace());
   Value *Name = buildNameForResource(BufDecl->getName(), CGM);
 
-  llvm::Intrinsic::ID IntrinsicID =
-      RBA->hasRegisterSlot()
-          ? CGM.getHLSLRuntime().getCreateHandleFromBindingIntrinsic()
-          : CGM.getHLSLRuntime().getCreateHandleFromImplicitBindingIntrinsic();
-
   // buffer with explicit binding
-  if (RBA->hasRegisterSlot()) {
-    auto *RegSlot = llvm::ConstantInt::get(CGM.IntTy, RBA->getSlotNumber());
+  if (Binding.isExplicit()) {
+    llvm::Intrinsic::ID IntrinsicID =
+        CGM.getHLSLRuntime().getCreateHandleFromBindingIntrinsic();
+    auto *RegSlot = llvm::ConstantInt::get(CGM.IntTy, Binding.getSlot());
     SmallVector<Value *> Args{Space, RegSlot, RangeSize, Index, Name};
     initializeBuffer(CGM, GV, IntrinsicID, Args);
   } else {
     // buffer with implicit binding
+    llvm::Intrinsic::ID IntrinsicID =
+        CGM.getHLSLRuntime().getCreateHandleFromImplicitBindingIntrinsic();
     auto *OrderID =
-        llvm::ConstantInt::get(CGM.IntTy, RBA->getImplicitBindingOrderID());
+        llvm::ConstantInt::get(CGM.IntTy, Binding.getImplicitOrderID());
     SmallVector<Value *> Args{OrderID, Space, RangeSize, Index, Name};
     initializeBuffer(CGM, GV, IntrinsicID, Args);
   }
@@ -960,9 +928,9 @@ std::optional<LValue> CGHLSLRuntime::emitResourceArraySubscriptExpr(
 
   // Find binding info for the resource array. For implicit binding
   // an HLSLResourceBindingAttr should have been added by SemaHLSL.
-  HLSLVkBindingAttr *VkBinding = ArrayDecl->getAttr<HLSLVkBindingAttr>();
-  HLSLResourceBindingAttr *RBA = ArrayDecl->getAttr<HLSLResourceBindingAttr>();
-  assert((VkBinding || RBA) && "resource array must have a binding attribute");
+  ResourceBindingAttrs Binding(ArrayDecl);
+  assert((Binding.hasBinding()) &&
+         "resource array must have a binding attribute");
 
   // Find the individual resource type.
   QualType ResultTy = ArraySubsExpr->getType();
@@ -992,7 +960,7 @@ std::optional<LValue> CGHLSLRuntime::emitResourceArraySubscriptExpr(
     CallArgList Args;
     CXXMethodDecl *CreateMethod = lookupResourceInitMethodAndSetupArgs(
         CGF.CGM, ResourceTy->getAsCXXRecordDecl(), Range, Index,
-        ArrayDecl->getName(), RBA, VkBinding, Args);
+        ArrayDecl->getName(), Binding, Args);
 
     if (!CreateMethod)
       // This can happen if someone creates an array of structs that looks like
@@ -1009,8 +977,8 @@ std::optional<LValue> CGHLSLRuntime::emitResourceArraySubscriptExpr(
         cast<ConstantArrayType>(ResultTy.getTypePtr());
     std::optional<llvm::Value *> EndIndex = initializeLocalResourceArray(
         CGF, ResourceTy->getAsCXXRecordDecl(), ArrayTy, ValueSlot, Range, Index,
-        ArrayDecl->getName(), RBA, VkBinding,
-        {llvm::ConstantInt::get(CGM.IntTy, 0)}, ArraySubsExpr->getExprLoc());
+        ArrayDecl->getName(), Binding, {llvm::ConstantInt::get(CGM.IntTy, 0)},
+        ArraySubsExpr->getExprLoc());
     if (!EndIndex)
       return std::nullopt;
   }
diff --git a/clang/lib/CodeGen/CGHLSLRuntime.h b/clang/lib/CodeGen/CGHLSLRuntime.h
index 9c0e6056fd4ee..7c6c2850fd4d4 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.h
+++ b/clang/lib/CodeGen/CGHLSLRuntime.h
@@ -200,11 +200,7 @@ class CGHLSLRuntime {
   void emitBufferGlobalsAndMetadata(const HLSLBufferDecl *BufDecl,
                                     llvm::GlobalVariable *BufGV);
   void initializeBufferFromBinding(const HLSLBufferDecl *BufDecl,
-                                   llvm::GlobalVariable *GV,
-                                   HLSLVkBindingAttr *VkBinding);
-  void initializeBufferFromBinding(const HLSLBufferDecl *BufDecl,
-                                   llvm::GlobalVariable *GV,
-                                   HLSLResourceBindingAttr *RBA);
+                                   llvm::GlobalVariable *GV);
   llvm::Triple::ArchType getArch();
 
   llvm::DenseMap<const clang::RecordType *, llvm::TargetExtType *> LayoutTypes;
diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index 940d510b4cc02..129b03c07c0bd 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -19,6 +19,7 @@
 #include "clang/AST/DeclarationName.h"
 #include "clang/AST/DynamicRecursiveASTVisitor.h"
 #include "clang/AST/Expr.h"
+#include "clang/AST/HLSLResource.h"
 #include "clang/AST/Type.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/Builtins.h"
@@ -52,6 +53,7 @@
 #include <utility>
 
 using namespace clang;
+using namespace clang::hlsl;
 using RegisterType = HLSLResourceBindingAttr::RegisterType;
 
 static CXXRecordDecl *createHostLayoutStruct(Sema &S,
@@ -3799,19 +3801,8 @@ bool SemaHLSL::initGlobalResourceDecl(VarDecl *VD) {
   uint64_t UIntTySize = AST.getTypeSize(AST.UnsignedIntTy);
   uint64_t IntTySize = AST.getTypeSize(AST.IntTy);
 
-  // Gather resource binding information from attributes.
-  HLSLResourceBindingAttr *RBA = VD->getAttr<HLSLResourceBindingAttr>();
-  HLSLVkBindingAttr *VkBinding = VD->getAttr<HLSLVkBindingAttr>();
-  std::optional<uint32_t> RegisterSlot;
-  uint32_t SpaceNo = 0;
-  if (VkBinding) {
-    RegisterSlot = VkBinding->getBinding();
-    SpaceNo = VkBinding->getSet();
-  } else if (RBA) {
-    if (RBA->hasRegisterSlot())
-      RegisterSlot = RBA->getSlotNumber();
-    SpaceNo = RBA->getSpaceNumber();
-  }
+  // Gather resource binding attributes.
+  ResourceBindingAttrs Binding(VD);
 
   // Find correct initialization method and create its arguments.
   QualType ResourceTy = VD->getType();
@@ -3819,21 +3810,21 @@ bool SemaHLSL::initGlobalResourceDecl(VarDecl *VD) {
   CXXMethodDecl *CreateMethod = nullptr;
   llvm::SmallVector<Expr *> Args;
 
-  if (RegisterSlot.has_value()) {
+  if (Binding.isExplicit()) {
     // The resource has explicit binding.
     CreateMethod = lookupMethod(SemaRef, ResourceDecl, "__createFromBinding",
                                 VD->getLocation());
-    IntegerLiteral *RegSlot = IntegerLiteral::Create(
-        AST, llvm::APInt(UIntTySize, RegisterSlot.value()), AST.UnsignedIntTy,
-        SourceLocation());
+    IntegerLiteral *RegSlot =
+        IntegerLiteral::Create(AST, llvm::APInt(UIntTySize, Binding.getSlot()),
+                               AST.UnsignedIntTy, SourceLocation());
     Args.push_back(RegSlot);
   } else {
     // The resource has implicit binding.
     CreateMethod =
         lookupMethod(SemaRef, ResourceDecl, "__createFromImplicitBinding",
                      VD->getLocation());
-    uint32_t OrderID = (RBA && RBA->hasImplicitBindingOrderID())
-                           ? RBA->getImplicitBindingOrderID()
+    uint32_t OrderID = (Binding.hasImplicitOrderID())
+                           ? Binding.getImplicitOrderID()
                            : getNextImplicitBindingOrderID();
     IntegerLiteral *OrderId =
         IntegerLiteral::Create(AST, llvm::APInt(UIntTySize, OrderID),
@@ -3848,7 +3839,7 @@ bool SemaHLSL::initGlobalResourceDecl(VarDecl *VD) {
     return false;
 
   IntegerLiteral *Space =
-      IntegerLiteral::Create(AST, llvm::APInt(UIntTySize, SpaceNo),
+      IntegerLiteral::Create(AST, llvm::APInt(UIntTySize, Binding.getSpace()),
                              AST.UnsignedIntTy, SourceLocation());
   Args.push_back(Space);
 

Copy link

github-actions bot commented Sep 29, 2025

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

Copy link
Contributor

@bob80905 bob80905 left a comment

Choose a reason for hiding this comment

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

My comment is a nit, this looks like a great simplification nevertheless!

ResourceBindingAttrs(const Decl *D) {
RegBinding = D->getAttr<HLSLResourceBindingAttr>();
bool IsSpirv = D->getASTContext().getTargetInfo().getTriple().isSPIRV();
VkBinding = IsSpirv ? D->getAttr<HLSLVkBindingAttr>() : nullptr;
Copy link
Contributor

@bob80905 bob80905 Sep 29, 2025

Choose a reason for hiding this comment

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

I think getAttr should return a nullptr if the attr isn't available, which should make IsSpirv and this ternary unnecessary.
i.e, consider VkBinding = D->getAttr<HLSLVkBindingAttr>();

Copy link
Member Author

Choose a reason for hiding this comment

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

There can still be [[vk::binding]] attribute in HLSL that is compiled for DirectX and we need to handle that.

BTW SemaHLSL currently does not add the parsed attribute to the AST if the target is DirectX, which is not correct. The attribute needs to be preserved in the AST as is even if it is not used for rewriter scenarios. I will fix that in a follow-up PR.

@s-perron
Copy link
Contributor

This is a nice clean up. Thanks.

@hekota hekota merged commit b6dfa3d into llvm:main Sep 30, 2025
10 checks passed
rupprecht added a commit that referenced this pull request Oct 1, 2025
…161473)

HLSLResource.h added by #161254 builds in the context of a .cpp file
(e.g. CGHLSLRuntime.cpp) but not when doing a header compilation, e.g.:

```
clang/include/clang/AST/Attrs.inc:12:45: error: unknown type name 'raw_ostream'; did you mean 'clang::raw_ostream'?
   12 | static inline void DelimitAttributeArgument(raw_ostream& OS, bool& IsFirst) {
```
mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Oct 3, 2025
…ng attributes (llvm#161254)

Add new `ResourceBindingAttrs` struct that holds resource binding attributes `HLSLResourceBindingAttr` and `HLSLVkBindingAttr` and provides helper methods to simplify dealing with resource bindings. This code is placed in the AST library to be shared between Sema and CodeGen.

This change has been done in preparation of a third binding attribute coming soon to represent `[[vk::counter_binding()]]`. This new attribute and more helper member functions will be added to `ResourceBindingAttrs` and will be used in both Sema and in CodeGen to implement resource counter initialization.
mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Oct 3, 2025
…lvm#161473)

HLSLResource.h added by llvm#161254 builds in the context of a .cpp file
(e.g. CGHLSLRuntime.cpp) but not when doing a header compilation, e.g.:

```
clang/include/clang/AST/Attrs.inc:12:45: error: unknown type name 'raw_ostream'; did you mean 'clang::raw_ostream'?
   12 | static inline void DelimitAttributeArgument(raw_ostream& OS, bool& IsFirst) {
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen IR generation bugs: mangling, exceptions, etc. 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.

4 participants