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@+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) {} @@ -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); 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@+1 {{resource has implicit register binding}} +RWBuffer<float> dd : register(space1); // No warning - explicit binding. RWBuffer<float> ddd : register(u3, space4); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
