[clang] 2a5ee25 - [HLSL] Allow resource annotations to specify only register space (#135287)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 30 10:27:30 PDT 2025
Author: Helena Kotas
Date: 2025-04-30T10:27:27-07:00
New Revision: 2a5ee2501d6249933b0dd4f81a4ec56e146b9669
URL: https://github.com/llvm/llvm-project/commit/2a5ee2501d6249933b0dd4f81a4ec56e146b9669
DIFF: https://github.com/llvm/llvm-project/commit/2a5ee2501d6249933b0dd4f81a4ec56e146b9669.diff
LOG: [HLSL] Allow resource annotations to specify only register space (#135287)
Specifying only `space` in a `register` annotation means the compiler
should implicitly assign a register slot to the resource from the
provided virtual register space.
Closes #133346
Added:
Modified:
clang/include/clang/Basic/Attr.td
clang/lib/CodeGen/CGHLSLRuntime.cpp
clang/lib/Parse/ParseHLSL.cpp
clang/lib/Sema/SemaHLSL.cpp
clang/test/AST/HLSL/resource_binding_attr.hlsl
clang/test/SemaHLSL/resource_binding_attr_error.hlsl
clang/test/SemaHLSL/resource_binding_implicit.hlsl
Removed:
################################################################################
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 3e426bb685924..1f42fcbecc811 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -4752,20 +4752,25 @@ def HLSLResourceBinding: InheritableAttr {
private:
RegisterType RegType;
- unsigned SlotNumber;
+ std::optional<unsigned> SlotNumber;
unsigned SpaceNumber;
public:
- void setBinding(RegisterType RT, unsigned SlotNum, unsigned SpaceNum) {
+ void setBinding(RegisterType RT, std::optional<unsigned> SlotNum, unsigned SpaceNum) {
RegType = RT;
SlotNumber = SlotNum;
SpaceNumber = SpaceNum;
}
+ bool isImplicit() const {
+ return !SlotNumber.has_value();
+ }
RegisterType getRegisterType() const {
+ assert(!isImplicit() && "binding does not have register slot");
return RegType;
}
unsigned getSlotNumber() const {
- return SlotNumber;
+ assert(!isImplicit() && "binding does not have register slot");
+ return SlotNumber.value();
}
unsigned getSpaceNumber() const {
return SpaceNumber;
diff --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp b/clang/lib/CodeGen/CGHLSLRuntime.cpp
index b787595068f75..6136417fcad60 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -261,7 +261,7 @@ void CGHLSLRuntime::addBuffer(const HLSLBufferDecl *BufDecl) {
BufDecl->getAttr<HLSLResourceBindingAttr>();
// FIXME: handle implicit binding if no binding attribute is found
// (llvm/llvm-project#110722)
- if (RBA)
+ if (RBA && !RBA->isImplicit())
initializeBufferFromBinding(CGM, BufGV, RBA->getSlotNumber(),
RBA->getSpaceNumber());
}
diff --git a/clang/lib/Parse/ParseHLSL.cpp b/clang/lib/Parse/ParseHLSL.cpp
index a36d324affc57..5569605c287b1 100644
--- a/clang/lib/Parse/ParseHLSL.cpp
+++ b/clang/lib/Parse/ParseHLSL.cpp
@@ -163,11 +163,16 @@ void Parser::ParseHLSLAnnotations(ParsedAttributes &Attrs,
SourceLocation SlotLoc = Tok.getLocation();
ArgExprs.push_back(ParseIdentifierLoc());
- // Add numeric_constant for fix-it.
- if (SlotStr.size() == 1 && Tok.is(tok::numeric_constant))
+ if (SlotStr.size() == 1) {
+ if (!Tok.is(tok::numeric_constant)) {
+ Diag(Tok.getLocation(), diag::err_expected) << tok::numeric_constant;
+ SkipUntil(tok::r_paren, StopAtSemi); // skip through )
+ return;
+ }
+ // Add numeric_constant for fix-it.
fixSeparateAttrArgAndNumber(SlotStr, SlotLoc, Tok, ArgExprs, *this,
Actions.Context, PP);
-
+ }
if (Tok.is(tok::comma)) {
ConsumeToken(); // consume comma
if (!Tok.is(tok::identifier)) {
diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index 1e6e23780ff76..f51bde4827ad1 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -1533,72 +1533,80 @@ void SemaHLSL::handleResourceBindingAttr(Decl *TheDecl, const ParsedAttr &AL) {
diag::err_incomplete_type))
return;
}
- StringRef Space = "space0";
+
StringRef Slot = "";
+ StringRef Space = "";
+ SourceLocation SlotLoc, SpaceLoc;
if (!AL.isArgIdent(0)) {
Diag(AL.getLoc(), diag::err_attribute_argument_type)
<< AL << AANT_ArgumentIdentifier;
return;
}
-
IdentifierLoc *Loc = AL.getArgAsIdent(0);
- StringRef Str = Loc->getIdentifierInfo()->getName();
- SourceLocation ArgLoc = Loc->getLoc();
- SourceLocation SpaceArgLoc;
- bool SpecifiedSpace = false;
if (AL.getNumArgs() == 2) {
- SpecifiedSpace = true;
- Slot = Str;
+ Slot = Loc->getIdentifierInfo()->getName();
+ SlotLoc = Loc->getLoc();
if (!AL.isArgIdent(1)) {
Diag(AL.getLoc(), diag::err_attribute_argument_type)
<< AL << AANT_ArgumentIdentifier;
return;
}
-
- IdentifierLoc *Loc = AL.getArgAsIdent(1);
+ Loc = AL.getArgAsIdent(1);
Space = Loc->getIdentifierInfo()->getName();
- SpaceArgLoc = Loc->getLoc();
+ SpaceLoc = Loc->getLoc();
} else {
- Slot = Str;
+ StringRef Str = Loc->getIdentifierInfo()->getName();
+ if (Str.starts_with("space")) {
+ Space = Str;
+ SpaceLoc = Loc->getLoc();
+ } else {
+ Slot = Str;
+ SlotLoc = Loc->getLoc();
+ Space = "space0";
+ }
}
- RegisterType RegType;
- unsigned SlotNum = 0;
+ RegisterType RegType = RegisterType::SRV;
+ std::optional<unsigned> SlotNum;
unsigned SpaceNum = 0;
- // Validate.
+ // Validate slot
if (!Slot.empty()) {
if (!convertToRegisterType(Slot, &RegType)) {
- Diag(ArgLoc, diag::err_hlsl_binding_type_invalid) << Slot.substr(0, 1);
+ Diag(SlotLoc, diag::err_hlsl_binding_type_invalid) << Slot.substr(0, 1);
return;
}
if (RegType == RegisterType::I) {
- Diag(ArgLoc, diag::warn_hlsl_deprecated_register_type_i);
+ Diag(SlotLoc, diag::warn_hlsl_deprecated_register_type_i);
return;
}
-
StringRef SlotNumStr = Slot.substr(1);
- if (SlotNumStr.getAsInteger(10, SlotNum)) {
- Diag(ArgLoc, diag::err_hlsl_unsupported_register_number);
+ unsigned N;
+ if (SlotNumStr.getAsInteger(10, N)) {
+ Diag(SlotLoc, diag::err_hlsl_unsupported_register_number);
return;
}
+ SlotNum = N;
}
+ // Validate space
if (!Space.starts_with("space")) {
- Diag(SpaceArgLoc, diag::err_hlsl_expected_space) << Space;
+ Diag(SpaceLoc, diag::err_hlsl_expected_space) << Space;
return;
}
StringRef SpaceNumStr = Space.substr(5);
if (SpaceNumStr.getAsInteger(10, SpaceNum)) {
- Diag(SpaceArgLoc, diag::err_hlsl_expected_space) << Space;
+ Diag(SpaceLoc, diag::err_hlsl_expected_space) << Space;
return;
}
- if (!DiagnoseHLSLRegisterAttribute(SemaRef, ArgLoc, TheDecl, RegType,
- SpecifiedSpace))
- return;
+ // If we have slot, diagnose it is the right register type for the decl
+ if (SlotNum.has_value())
+ if (!DiagnoseHLSLRegisterAttribute(SemaRef, SlotLoc, TheDecl, RegType,
+ !SpaceLoc.isInvalid()))
+ return;
HLSLResourceBindingAttr *NewAttr =
HLSLResourceBindingAttr::Create(getASTContext(), Slot, Space, AL);
@@ -1971,7 +1979,7 @@ void SemaHLSL::ActOnEndOfTranslationUnit(TranslationUnitDecl *TU) {
for (const Decl *VD : DefaultCBufferDecls) {
const HLSLResourceBindingAttr *RBA =
VD->getAttr<HLSLResourceBindingAttr>();
- if (RBA &&
+ if (RBA && !RBA->isImplicit() &&
RBA->getRegisterType() == HLSLResourceBindingAttr::RegisterType::C) {
DefaultCBuffer->setHasValidPackoffset(true);
break;
@@ -3262,7 +3270,7 @@ static bool initVarDeclWithCtor(Sema &S, VarDecl *VD,
static bool initGlobalResourceDecl(Sema &S, VarDecl *VD) {
HLSLResourceBindingAttr *RBA = VD->getAttr<HLSLResourceBindingAttr>();
- if (!RBA)
+ if (!RBA || RBA->isImplicit())
// FIXME: add support for implicit binding (llvm/llvm-project#110722)
return false;
@@ -3347,7 +3355,7 @@ void SemaHLSL::processExplicitBindingsOnDecl(VarDecl *VD) {
bool HasBinding = false;
for (Attr *A : VD->attrs()) {
HLSLResourceBindingAttr *RBA = dyn_cast<HLSLResourceBindingAttr>(A);
- if (!RBA)
+ if (!RBA || RBA->isImplicit())
continue;
HasBinding = true;
diff --git a/clang/test/AST/HLSL/resource_binding_attr.hlsl b/clang/test/AST/HLSL/resource_binding_attr.hlsl
index af43eddc45edd..ef75919fc3daf 100644
--- a/clang/test/AST/HLSL/resource_binding_attr.hlsl
+++ b/clang/test/AST/HLSL/resource_binding_attr.hlsl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -finclude-default-header -ast-dump -o - %s | FileCheck %s
+// RUN: %clang_cc1 -Wno-hlsl-implicit-binding -triple dxil-pc-shadermodel6.3-library -finclude-default-header -ast-dump -o - %s | FileCheck %s
// CHECK: HLSLBufferDecl {{.*}} line:[[# @LINE + 4]]:9 cbuffer CB
// CHECK-NEXT: HLSLResourceClassAttr {{.*}} Implicit CBuffer
@@ -30,6 +30,10 @@ RWBuffer<float> UAV : register(u3);
// CHECK: HLSLResourceBindingAttr {{.*}} "u4" "space0"
RWBuffer<float> UAV1 : register(u2), UAV2 : register(u4);
+// CHECK: VarDecl {{.*}} UAV3 'RWBuffer<float>':'hlsl::RWBuffer<float>'
+// CHECK: HLSLResourceBindingAttr {{.*}} "" "space5"
+RWBuffer<float> UAV3 : register(space5);
+
//
// Default constants ($Globals) layout annotations
diff --git a/clang/test/SemaHLSL/resource_binding_attr_error.hlsl b/clang/test/SemaHLSL/resource_binding_attr_error.hlsl
index a3a91c3ddddb8..b91c2efba167a 100644
--- a/clang/test/SemaHLSL/resource_binding_attr_error.hlsl
+++ b/clang/test/SemaHLSL/resource_binding_attr_error.hlsl
@@ -22,8 +22,8 @@ cbuffer c : register(bf, s2) {
// expected-error at +1 {{expected identifier}}
cbuffer A : register() {}
-// expected-error at +1 {{register number should be an integer}}
-cbuffer B : register(space1) {}
+// expected-error at +1 {{invalid space specifier 'space' used; expected 'space' followed by an integer, like space1}}
+cbuffer B : register(space) {}
// expected-error at +1 {{wrong argument format for hlsl attribute, use b2 instead}}
cbuffer C : register(b 2) {}
@@ -31,6 +31,9 @@ cbuffer C : register(b 2) {}
// expected-error at +1 {{wrong argument format for hlsl attribute, use b2 instead}}
cbuffer D : register(b 2, space3) {}
+// expected-error at +1 {{expected <numeric_constant>}}
+cbuffer E : register(u-1) {};
+
// expected-error at +1 {{'register' attribute only applies to cbuffer/tbuffer and external global variables}}
static MyTemplatedSRV<float> U : register(u5);
diff --git a/clang/test/SemaHLSL/resource_binding_implicit.hlsl b/clang/test/SemaHLSL/resource_binding_implicit.hlsl
index 8f0e721c7153f..ce9f0d1ac364f 100644
--- a/clang/test/SemaHLSL/resource_binding_implicit.hlsl
+++ b/clang/test/SemaHLSL/resource_binding_implicit.hlsl
@@ -14,9 +14,8 @@ RWBuffer<int> c;
// No warning - explicit binding.
RWBuffer<float> d : register(u0);
-// TODO: Add this test once #135287 lands
-// TODO: ... @+1 {{resource has implicit register binding}}
-// TODO: RWBuffer<float> dd : register(space1);
+// expected-warning at +1 {{resource has implicit register binding}}
+RWBuffer<float> dd : register(space1);
// No warning - explicit binding.
RWBuffer<float> ddd : register(u3, space4);
More information about the cfe-commits
mailing list