https://github.com/hekota updated
https://github.com/llvm/llvm-project/pull/135287
>From 6fb658a603f116f29e635e4802a8d77b896150ff Mon Sep 17 00:00:00 2001
From: Helena Kotas <heko...@microsoft.com>
Date: Thu, 10 Apr 2025 17:31:57 -0700
Subject: [PATCH 1/4] [HLSL] Allow register annotations to specify only `space`
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.
---
clang/include/clang/Basic/Attr.td | 11 ++-
clang/lib/CodeGen/CGHLSLRuntime.cpp | 2 +-
clang/lib/Sema/SemaHLSL.cpp | 74 +++++++++++--------
.../test/AST/HLSL/resource_binding_attr.hlsl | 4 +
.../SemaHLSL/resource_binding_attr_error.hlsl | 4 +-
test.ll | 7 ++
6 files changed, 64 insertions(+), 38 deletions(-)
create mode 100644 test.ll
diff --git a/clang/include/clang/Basic/Attr.td
b/clang/include/clang/Basic/Attr.td
index fd9e686485552..1fe37ad4e2d4d 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -4751,20 +4751,25 @@ def HLSLResourceBinding: InheritableAttr {
private:
RegisterType RegType;
- unsigned SlotNumber;
+ int SlotNumber; // -1 if the register slot was not specified
unsigned SpaceNumber;
public:
- void setBinding(RegisterType RT, unsigned SlotNum, unsigned SpaceNum) {
+ void setBinding(RegisterType RT, int SlotNum, unsigned SpaceNum) {
RegType = RT;
SlotNumber = SlotNum;
SpaceNumber = SpaceNum;
}
+ bool isImplicit() const {
+ return SlotNumber < 0;
+ }
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 (unsigned)SlotNumber;
}
unsigned getSpaceNumber() const {
return SpaceNumber;
diff --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp
b/clang/lib/CodeGen/CGHLSLRuntime.cpp
index 450213fcec676..e42bb8e16e80b 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/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index 27959f61f1dc3..73b8c19840026 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -1529,72 +1529,82 @@ 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->Ident->getName();
- SourceLocation ArgLoc = Loc->Loc;
- SourceLocation SpaceArgLoc;
- bool SpecifiedSpace = false;
if (AL.getNumArgs() == 2) {
- SpecifiedSpace = true;
- Slot = Str;
+ Slot = Loc->Ident->getName();
+ SlotLoc = Loc->Loc;
+
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->Ident->getName();
- SpaceArgLoc = Loc->Loc;
+ SpaceLoc = Loc->Loc;
} else {
- Slot = Str;
+ StringRef Str = Loc->Ident->getName();
+ if (Str.starts_with("space")) {
+ Space = Str;
+ SpaceLoc = Loc->Loc;
+ } else {
+ Slot = Str;
+ SlotLoc = Loc->Loc;
+ Space = "space0";
+ }
}
- RegisterType RegType;
- unsigned SlotNum = 0;
+ RegisterType RegType = RegisterType::SRV;
+ int SlotNum = -1;
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);
+ Diag(SlotLoc, diag::err_hlsl_unsupported_register_number);
return;
}
}
- if (!Space.starts_with("space")) {
- Diag(SpaceArgLoc, 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;
- return;
+ // Validate space
+ if (!Space.empty()) {
+ if (!Space.starts_with("space")) {
+ Diag(SpaceLoc, diag::err_hlsl_expected_space) << Space;
+ return;
+ }
+ StringRef SpaceNumStr = Space.substr(5);
+ if (SpaceNumStr.getAsInteger(10, SpaceNum)) {
+ 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 >= 0)
+ if (!DiagnoseHLSLRegisterAttribute(SemaRef, SlotLoc, TheDecl, RegType,
+ !SpaceLoc.isInvalid()))
+ return;
HLSLResourceBindingAttr *NewAttr =
HLSLResourceBindingAttr::Create(getASTContext(), Slot, Space, AL);
@@ -1967,7 +1977,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;
@@ -3227,7 +3237,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;
@@ -3310,7 +3320,7 @@ void SemaHLSL::processExplicitBindingsOnDecl(VarDecl *VD)
{
for (Attr *A : VD->attrs()) {
HLSLResourceBindingAttr *RBA = dyn_cast<HLSLResourceBindingAttr>(A);
- if (!RBA)
+ if (!RBA || RBA->isImplicit())
continue;
RegisterType RT = RBA->getRegisterType();
diff --git a/clang/test/AST/HLSL/resource_binding_attr.hlsl
b/clang/test/AST/HLSL/resource_binding_attr.hlsl
index af43eddc45edd..c073cd4dc1476 100644
--- a/clang/test/AST/HLSL/resource_binding_attr.hlsl
+++ b/clang/test/AST/HLSL/resource_binding_attr.hlsl
@@ -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 74aff79f0e37f..5f0ab66061315 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@+1 {{expected identifier}}
cbuffer A : register() {}
-// expected-error@+1 {{register number should be an integer}}
-cbuffer B : register(space1) {}
+// expected-error@+1 {{invalid space specifier 'space' used; expected 'space'
followed by an integer, like space1}}
+cbuffer B : register(space) {}
// expected-error@+1 {{wrong argument format for hlsl attribute, use b2
instead}}
cbuffer C : register(b 2) {}
diff --git a/test.ll b/test.ll
new file mode 100644
index 0000000000000..dc674289e3e74
--- /dev/null
+++ b/test.ll
@@ -0,0 +1,7 @@
+ByteAddressBuffer Buffer0: register(t0);
+RWBuffer<float> Buf : register(u0);
+
+[numthreads(4,1,1)]
+void main() {
+ Buf[0] = 10;
+}
>From c5c7ab8ace5d82a3f84c5b00738b1d7fd3b3b311 Mon Sep 17 00:00:00 2001
From: Helena Kotas <heko...@microsoft.com>
Date: Thu, 10 Apr 2025 18:13:35 -0700
Subject: [PATCH 2/4] remove condition - Space is never empty
---
clang/lib/Sema/SemaHLSL.cpp | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index 73b8c19840026..786bfcb3acf7a 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -1588,16 +1588,14 @@ void SemaHLSL::handleResourceBindingAttr(Decl *TheDecl,
const ParsedAttr &AL) {
}
// Validate space
- if (!Space.empty()) {
- if (!Space.starts_with("space")) {
- Diag(SpaceLoc, diag::err_hlsl_expected_space) << Space;
- return;
- }
- StringRef SpaceNumStr = Space.substr(5);
- if (SpaceNumStr.getAsInteger(10, SpaceNum)) {
- Diag(SpaceLoc, diag::err_hlsl_expected_space) << Space;
- return;
- }
+ if (!Space.starts_with("space")) {
+ Diag(SpaceLoc, diag::err_hlsl_expected_space) << Space;
+ return;
+ }
+ StringRef SpaceNumStr = Space.substr(5);
+ if (SpaceNumStr.getAsInteger(10, SpaceNum)) {
+ Diag(SpaceLoc, diag::err_hlsl_expected_space) << Space;
+ return;
}
// If we have slot, diagnose it is the right register type for the decl
>From 7084cf1978d01f1d4295e3facacf667b3f020fb1 Mon Sep 17 00:00:00 2001
From: Helena Kotas <heko...@microsoft.com>
Date: Thu, 10 Apr 2025 18:16:31 -0700
Subject: [PATCH 3/4] remove unintentionally added file
---
test.ll | 7 -------
1 file changed, 7 deletions(-)
delete mode 100644 test.ll
diff --git a/test.ll b/test.ll
deleted file mode 100644
index dc674289e3e74..0000000000000
--- a/test.ll
+++ /dev/null
@@ -1,7 +0,0 @@
-ByteAddressBuffer Buffer0: register(t0);
-RWBuffer<float> Buf : register(u0);
-
-[numthreads(4,1,1)]
-void main() {
- Buf[0] = 10;
-}
>From b79e4ddae3485511dd75038a1e2be8210d041969 Mon Sep 17 00:00:00 2001
From: Helena Kotas <heko...@microsoft.com>
Date: Fri, 11 Apr 2025 12:21:09 -0700
Subject: [PATCH 4/4] Handle register(u-1);
---
clang/lib/Parse/ParseHLSL.cpp | 11 ++++++++---
clang/test/SemaHLSL/resource_binding_attr_error.hlsl | 3 +++
2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/clang/lib/Parse/ParseHLSL.cpp b/clang/lib/Parse/ParseHLSL.cpp
index f4c109f9a81a2..6e8fd56fe5262 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/test/SemaHLSL/resource_binding_attr_error.hlsl
b/clang/test/SemaHLSL/resource_binding_attr_error.hlsl
index 5f0ab66061315..d126d25bd94f0 100644
--- a/clang/test/SemaHLSL/resource_binding_attr_error.hlsl
+++ b/clang/test/SemaHLSL/resource_binding_attr_error.hlsl
@@ -31,6 +31,9 @@ cbuffer C : register(b 2) {}
// expected-error@+1 {{wrong argument format for hlsl attribute, use b2
instead}}
cbuffer D : register(b 2, space3) {}
+// expected-error@+1 {{expected <numeric_constant>}}
+cbuffer E : register(u-1) {};
+
// expected-error@+1 {{'register' attribute only applies to cbuffer/tbuffer
and external global variables}}
static MyTemplatedSRV<float> U : register(u5);
_______________________________________________
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
