From 6785ed1b738b6e54f9488205da6fb2f923a38413 Mon Sep 17 00:00:00 2001 From: Helena Kotas Date: Mon, 29 Sep 2025 11:15:35 -0700 Subject: [PATCH 1/3] [HLSL][NFC] Add ResourceBindingAttrs to simplity dealing with resource 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. --- clang/include/clang/AST/HLSLResource.h | 75 ++++++++++++++++++++ clang/lib/CodeGen/CGHLSLRuntime.cpp | 94 +++++++++----------------- clang/lib/CodeGen/CGHLSLRuntime.h | 6 +- clang/lib/Sema/SemaHLSL.cpp | 31 +++------ 4 files changed, 118 insertions(+), 88 deletions(-) create mode 100644 clang/include/clang/AST/HLSLResource.h 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(); + bool IsSpirv = D->getASTContext().getTargetInfo().getTriple().isSPIRV(); + VkBinding = IsSpirv ? D->getAttr() : 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 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 initializeLocalResourceArray( CodeGenFunction &CGF, CXXRecordDecl *ResourceDecl, const ConstantArrayType *ArrayTy, AggValueSlot &ValueSlot, llvm::Value *Range, llvm::Value *StartIndex, StringRef ResourceName, - HLSLResourceBindingAttr *RBA, HLSLVkBindingAttr *VkBinding, - ArrayRef PrevGEPIndices, SourceLocation ArraySubsExprLoc) { + ResourceBindingAttrs &Binding, ArrayRef PrevGEPIndices, + SourceLocation ArraySubsExprLoc) { ASTContext &AST = CGF.getContext(); llvm::IntegerType *IntTy = CGF.CGM.IntTy; @@ -220,7 +210,7 @@ static std::optional initializeLocalResourceArray( } std::optional 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 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()) { - initializeBufferFromBinding(BufDecl, BufGV, VkBinding); - } else { - HLSLResourceBindingAttr *RBA = BufDecl->getAttr(); - 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 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 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 Args{OrderID, Space, RangeSize, Index, Name}; initializeBuffer(CGM, GV, IntrinsicID, Args); } @@ -960,9 +928,9 @@ std::optional CGHLSLRuntime::emitResourceArraySubscriptExpr( // Find binding info for the resource array. For implicit binding // an HLSLResourceBindingAttr should have been added by SemaHLSL. - HLSLVkBindingAttr *VkBinding = ArrayDecl->getAttr(); - HLSLResourceBindingAttr *RBA = ArrayDecl->getAttr(); - 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 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 CGHLSLRuntime::emitResourceArraySubscriptExpr( cast(ResultTy.getTypePtr()); std::optional 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 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 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(); - HLSLVkBindingAttr *VkBinding = VD->getAttr(); - std::optional 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 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); From a803941c90907700d062a32f60f9cf2c76b573a5 Mon Sep 17 00:00:00 2001 From: Helena Kotas Date: Mon, 29 Sep 2025 11:27:16 -0700 Subject: [PATCH 2/3] clang-format --- clang/lib/CodeGen/CGHLSLRuntime.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp b/clang/lib/CodeGen/CGHLSLRuntime.cpp index ebe6d605812fd..c3fdd9840d078 100644 --- a/clang/lib/CodeGen/CGHLSLRuntime.cpp +++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp @@ -234,8 +234,7 @@ static std::optional initializeLocalResourceArray( CallArgList Args; CXXMethodDecl *CreateMethod = lookupResourceInitMethodAndSetupArgs( - CGF.CGM, ResourceDecl, Range, Index, ResourceName, Binding, - Args); + CGF.CGM, ResourceDecl, Range, Index, ResourceName, Binding, Args); if (!CreateMethod) // This can happen if someone creates an array of structs that looks like From 4b5e69dd2ff62079224901837b1d13a2634e3f33 Mon Sep 17 00:00:00 2001 From: Helena Kotas Date: Mon, 29 Sep 2025 11:36:19 -0700 Subject: [PATCH 3/3] clang-format - reorder includes --- clang/lib/CodeGen/CGHLSLRuntime.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp b/clang/lib/CodeGen/CGHLSLRuntime.cpp index c3fdd9840d078..ede1780592bf5 100644 --- a/clang/lib/CodeGen/CGHLSLRuntime.cpp +++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp @@ -21,8 +21,8 @@ #include "clang/AST/ASTContext.h" #include "clang/AST/Attrs.inc" #include "clang/AST/Decl.h" -#include "clang/AST/RecursiveASTVisitor.h" #include "clang/AST/HLSLResource.h" +#include "clang/AST/RecursiveASTVisitor.h" #include "clang/AST/Type.h" #include "clang/Basic/TargetOptions.h" #include "clang/Frontend/FrontendDiagnostic.h"