llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-codegen Author: Helena Kotas (hekota) <details> <summary>Changes</summary> 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. Fixes #<!-- -->133346 Depends on PR #<!-- -->135120 --- Full diff: https://github.com/llvm/llvm-project/pull/135287.diff 5 Files Affected: - (modified) clang/include/clang/Basic/Attr.td (+8-3) - (modified) clang/lib/CodeGen/CGHLSLRuntime.cpp (+1-1) - (modified) clang/lib/Sema/SemaHLSL.cpp (+34-26) - (modified) clang/test/AST/HLSL/resource_binding_attr.hlsl (+4) - (modified) clang/test/SemaHLSL/resource_binding_attr_error.hlsl (+2-2) ``````````diff 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..786bfcb3acf7a 100644 --- a/clang/lib/Sema/SemaHLSL.cpp +++ b/clang/lib/Sema/SemaHLSL.cpp @@ -1529,72 +1529,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->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; } } + // 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 >= 0) + if (!DiagnoseHLSLRegisterAttribute(SemaRef, SlotLoc, TheDecl, RegType, + !SpaceLoc.isInvalid())) + return; HLSLResourceBindingAttr *NewAttr = HLSLResourceBindingAttr::Create(getASTContext(), Slot, Space, AL); @@ -1967,7 +1975,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 +3235,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 +3318,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) {} `````````` </details> https://github.com/llvm/llvm-project/pull/135287 _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
