https://github.com/bob80905 updated https://github.com/llvm/llvm-project/pull/106782
>From 99408f31a8946df7ef9efa223d0dba2ab876fcd0 Mon Sep 17 00:00:00 2001 From: Joshua Batista <jbati...@microsoft.com> Date: Fri, 30 Aug 2024 12:08:37 -0700 Subject: [PATCH 1/4] add diag and testing for space and global constants --- .../clang/Basic/DiagnosticSemaKinds.td | 1 + clang/lib/Sema/SemaHLSL.cpp | 16 +++++++++--- .../SemaHLSL/resource_binding_attr_error.hlsl | 17 +++---------- .../resource_binding_attr_error_basic.hlsl | 21 +++++++++------- .../resource_binding_attr_error_space.hlsl | 25 +++++++++++++++++++ 5 files changed, 53 insertions(+), 27 deletions(-) create mode 100644 clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index edf22b909c4d57..e099cb954cac75 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -12357,6 +12357,7 @@ def warn_hlsl_deprecated_register_type_b: Warning<"binding type 'b' only applies def warn_hlsl_deprecated_register_type_i: Warning<"binding type 'i' ignored. The 'integer constant' binding type is no longer supported">, InGroup<LegacyConstantRegisterBinding>, DefaultError; def err_hlsl_unsupported_register_number : Error<"register number should be an integer">; def err_hlsl_expected_space : Error<"invalid space specifier '%0' used; expected 'space' followed by an integer, like space1">; +def err_hlsl_space_on_global_constant : Error<"register space cannot be specified on global constants">; def warn_hlsl_packoffset_mix : Warning<"cannot mix packoffset elements with nonpackoffset elements in a cbuffer">, InGroup<HLSLMixPackOffset>; def err_hlsl_packoffset_overlap : Error<"packoffset overlap between %0, %1">; diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp index 714e8f5cfa9926..61bed66445dc78 100644 --- a/clang/lib/Sema/SemaHLSL.cpp +++ b/clang/lib/Sema/SemaHLSL.cpp @@ -722,7 +722,8 @@ static void ValidateMultipleRegisterAnnotations(Sema &S, Decl *TheDecl, } static void DiagnoseHLSLRegisterAttribute(Sema &S, SourceLocation &ArgLoc, - Decl *TheDecl, RegisterType regType) { + Decl *TheDecl, RegisterType regType, + StringRef SpaceNum) { // Samplers, UAVs, and SRVs are VarDecl types VarDecl *TheVarDecl = dyn_cast<VarDecl>(TheDecl); @@ -795,6 +796,9 @@ static void DiagnoseHLSLRegisterAttribute(Sema &S, SourceLocation &ArgLoc, S.Diag(ArgLoc, diag::warn_hlsl_deprecated_register_type_b); else if (regType != RegisterType::C) S.Diag(ArgLoc, diag::err_hlsl_binding_type_mismatch) << regTypeNum; + // non-zero SpaceNum cannot be specified for global constants + if (SpaceNum != "0") + S.Diag(ArgLoc, diag::err_hlsl_space_on_global_constant); return; } @@ -817,8 +821,12 @@ static void DiagnoseHLSLRegisterAttribute(Sema &S, SourceLocation &ArgLoc, S.Diag(TheDecl->getLocation(), diag::warn_hlsl_user_defined_type_missing_member) << regTypeNum; - - return; + // non-zero SpaceNum cannot be specified for global constants + if (!isDeclaredWithinCOrTBuffer(TheDecl)) { + if (SpaceNum != "0") + S.Diag(ArgLoc, diag::err_hlsl_space_on_global_constant); + return; + } } } @@ -891,7 +899,7 @@ void SemaHLSL::handleResourceBindingAttr(Decl *TheDecl, const ParsedAttr &AL) { return; } - DiagnoseHLSLRegisterAttribute(SemaRef, ArgLoc, TheDecl, regType); + DiagnoseHLSLRegisterAttribute(SemaRef, ArgLoc, TheDecl, regType, SpaceNum); HLSLResourceBindingAttr *NewAttr = HLSLResourceBindingAttr::Create(getASTContext(), Slot, Space, AL); diff --git a/clang/test/SemaHLSL/resource_binding_attr_error.hlsl b/clang/test/SemaHLSL/resource_binding_attr_error.hlsl index 6a0b5956545dd8..823b75f14d1957 100644 --- a/clang/test/SemaHLSL/resource_binding_attr_error.hlsl +++ b/clang/test/SemaHLSL/resource_binding_attr_error.hlsl @@ -13,16 +13,9 @@ float a : register(c0); cbuffer b : register(i0) { } -// expected-error@+1 {{invalid space specifier 's2' used; expected 'space' followed by an integer, like space1}} -cbuffer c : register(b0, s2) { -} // expected-error@+1 {{register number should be an integer}} -cbuffer d : register(bf, s2) { - -} -// expected-error@+1 {{invalid space specifier 'spaces' used; expected 'space' followed by an integer, like space1}} -cbuffer e : register(b2, spaces) { +cbuffer c : register(bf, s2) { } @@ -35,9 +28,8 @@ cbuffer B : register(space1) {} // expected-error@+1 {{wrong argument format for hlsl attribute, use b2 instead}} cbuffer C : register(b 2) {} -// expected-error@+2 {{wrong argument format for hlsl attribute, use b2 instead}} -// expected-error@+1 {{wrong argument format for hlsl attribute, use space3 instead}} -cbuffer D : register(b 2, space 3) {} +// expected-error@+1 {{wrong argument format for hlsl attribute, use b2 instead}} +cbuffer D : register(b 2, space3) {} // expected-error@+1 {{'register' attribute only applies to cbuffer/tbuffer and external global variables}} static MyTemplatedSRV<float> U : register(u5); @@ -61,9 +53,6 @@ void foo2() { extern MyTemplatedSRV<float> U2 : register(u5); } -// expected-error@+1 {{binding type 'u' only applies to UAV resources}} -float b : register(u0, space1); - // expected-error@+1 {{'register' attribute only applies to cbuffer/tbuffer and external global variables}} void bar(MyTemplatedSRV<float> U : register(u3)) { diff --git a/clang/test/SemaHLSL/resource_binding_attr_error_basic.hlsl b/clang/test/SemaHLSL/resource_binding_attr_error_basic.hlsl index 0a547ed66af0a2..760c057630a7fa 100644 --- a/clang/test/SemaHLSL/resource_binding_attr_error_basic.hlsl +++ b/clang/test/SemaHLSL/resource_binding_attr_error_basic.hlsl @@ -3,37 +3,40 @@ // expected-error@+1{{binding type 't' only applies to SRV resources}} float f1 : register(t0); - -float f2 : register(c0); +// expected-error@+1 {{binding type 'u' only applies to UAV resources}} +float f2 : register(u0); // expected-error@+1{{binding type 'b' only applies to constant buffers. The 'bool constant' binding type is no longer supported}} float f3 : register(b9); +// expected-error@+1 {{binding type 's' only applies to sampler state}} +float f4 : register(s0); + // expected-error@+1{{binding type 'i' ignored. The 'integer constant' binding type is no longer supported}} -float f4 : register(i9); +float f5 : register(i9); // expected-error@+1{{binding type 'x' is invalid}} -float f5 : register(x9); +float f6 : register(x9); cbuffer g_cbuffer1 { // expected-error@+1{{binding type 'c' ignored in buffer declaration. Did you mean 'packoffset'?}} - float f6 : register(c2); + float f7 : register(c2); }; tbuffer g_tbuffer1 { // expected-error@+1{{binding type 'c' ignored in buffer declaration. Did you mean 'packoffset'?}} - float f7 : register(c2); + float f8 : register(c2); }; cbuffer g_cbuffer2 { // expected-error@+1{{binding type 'b' only applies to constant buffer resources}} - float f8 : register(b2); + float f9 : register(b2); }; tbuffer g_tbuffer2 { // expected-error@+1{{binding type 'i' ignored. The 'integer constant' binding type is no longer supported}} - float f9 : register(i2); + float f10 : register(i2); }; // expected-error@+1{{binding type 'c' only applies to numeric variables in the global scope}} -RWBuffer<float> f10 : register(c3); +RWBuffer<float> f11 : register(c3); diff --git a/clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl b/clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl new file mode 100644 index 00000000000000..2f1b19e1bfc696 --- /dev/null +++ b/clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl @@ -0,0 +1,25 @@ +// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -o - -fsyntax-only %s -verify + + +// expected-error@+1 {{invalid space specifier 's2' used; expected 'space' followed by an integer, like space1}} +cbuffer a : register(b0, s2) { + +} + +// expected-error@+1 {{invalid space specifier 'spaces' used; expected 'space' followed by an integer, like space1}} +cbuffer b : register(b2, spaces) { + +} + +// expected-error@+1 {{wrong argument format for hlsl attribute, use space3 instead}} +cbuffer c : register(b2, space 3) {} + +// expected-error@+1 {{register space cannot be specified on global constants}} +int d : register(c2, space3); + +struct S { + RWBuffer<int> a; +}; + +// expected-error@+1 {{register space cannot be specified on global constants}} +S thing : register(u2, space2); \ No newline at end of file >From 9fb3c96ef1027422953e9d2a434723ff3481c5df Mon Sep 17 00:00:00 2001 From: Joshua Batista <jbati...@microsoft.com> Date: Fri, 30 Aug 2024 12:23:20 -0700 Subject: [PATCH 2/4] add newline --- clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl b/clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl index 2f1b19e1bfc696..55f1fa50fac763 100644 --- a/clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl +++ b/clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl @@ -22,4 +22,4 @@ struct S { }; // expected-error@+1 {{register space cannot be specified on global constants}} -S thing : register(u2, space2); \ No newline at end of file +S thing : register(u2, space2); >From 59218ac2b3a5c1102060b32ba22eae36094db303 Mon Sep 17 00:00:00 2001 From: Joshua Batista <jbati...@microsoft.com> Date: Fri, 30 Aug 2024 13:38:55 -0700 Subject: [PATCH 3/4] detect space param properly, add more tests --- clang/lib/Sema/SemaHLSL.cpp | 12 ++++++------ .../SemaHLSL/resource_binding_attr_error_space.hlsl | 12 ++++++++++++ 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp index 61bed66445dc78..6fb5bbd3db961a 100644 --- a/clang/lib/Sema/SemaHLSL.cpp +++ b/clang/lib/Sema/SemaHLSL.cpp @@ -723,7 +723,7 @@ static void ValidateMultipleRegisterAnnotations(Sema &S, Decl *TheDecl, static void DiagnoseHLSLRegisterAttribute(Sema &S, SourceLocation &ArgLoc, Decl *TheDecl, RegisterType regType, - StringRef SpaceNum) { + const ParsedAttr &AL) { // Samplers, UAVs, and SRVs are VarDecl types VarDecl *TheVarDecl = dyn_cast<VarDecl>(TheDecl); @@ -796,8 +796,8 @@ static void DiagnoseHLSLRegisterAttribute(Sema &S, SourceLocation &ArgLoc, S.Diag(ArgLoc, diag::warn_hlsl_deprecated_register_type_b); else if (regType != RegisterType::C) S.Diag(ArgLoc, diag::err_hlsl_binding_type_mismatch) << regTypeNum; - // non-zero SpaceNum cannot be specified for global constants - if (SpaceNum != "0") + // Space argument cannot be specified for global constants + if (AL.getNumArgs() == 2) S.Diag(ArgLoc, diag::err_hlsl_space_on_global_constant); return; } @@ -821,9 +821,9 @@ static void DiagnoseHLSLRegisterAttribute(Sema &S, SourceLocation &ArgLoc, S.Diag(TheDecl->getLocation(), diag::warn_hlsl_user_defined_type_missing_member) << regTypeNum; - // non-zero SpaceNum cannot be specified for global constants + // Space argument cannot be specified for global constants if (!isDeclaredWithinCOrTBuffer(TheDecl)) { - if (SpaceNum != "0") + if (AL.getNumArgs() == 2) S.Diag(ArgLoc, diag::err_hlsl_space_on_global_constant); return; } @@ -899,7 +899,7 @@ void SemaHLSL::handleResourceBindingAttr(Decl *TheDecl, const ParsedAttr &AL) { return; } - DiagnoseHLSLRegisterAttribute(SemaRef, ArgLoc, TheDecl, regType, SpaceNum); + DiagnoseHLSLRegisterAttribute(SemaRef, ArgLoc, TheDecl, regType, AL); HLSLResourceBindingAttr *NewAttr = HLSLResourceBindingAttr::Create(getASTContext(), Slot, Space, AL); diff --git a/clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl b/clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl index 55f1fa50fac763..892839384302ec 100644 --- a/clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl +++ b/clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl @@ -17,6 +17,18 @@ cbuffer c : register(b2, space 3) {} // expected-error@+1 {{register space cannot be specified on global constants}} int d : register(c2, space3); +// expected-error@+1 {{register space cannot be specified on global constants}} +int e : register(c2, space0); + +// expected-error@+1 {{register space cannot be specified on global constants}} +int f : register(c2, space00); + +// valid +RWBuffer<int> g : register(u2, space0); + +// valid +RWBuffer<int> h : register(u2, space0); + struct S { RWBuffer<int> a; }; >From 1c1943db597e762162fb01773fc1045a15c8e54a Mon Sep 17 00:00:00 2001 From: Joshua Batista <jbati...@microsoft.com> Date: Thu, 12 Sep 2024 17:02:03 -0700 Subject: [PATCH 4/4] address damyan, simplify logic --- clang/lib/Sema/SemaHLSL.cpp | 51 ++++++++++--------- .../resource_binding_attr_error_space.hlsl | 19 +++++++ 2 files changed, 45 insertions(+), 25 deletions(-) diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp index 6fb5bbd3db961a..eb516898f6586c 100644 --- a/clang/lib/Sema/SemaHLSL.cpp +++ b/clang/lib/Sema/SemaHLSL.cpp @@ -722,8 +722,8 @@ static void ValidateMultipleRegisterAnnotations(Sema &S, Decl *TheDecl, } static void DiagnoseHLSLRegisterAttribute(Sema &S, SourceLocation &ArgLoc, - Decl *TheDecl, RegisterType regType, - const ParsedAttr &AL) { + Decl *TheDecl, RegisterType RegType, + const bool SpecifiedSpace) { // Samplers, UAVs, and SRVs are VarDecl types VarDecl *TheVarDecl = dyn_cast<VarDecl>(TheDecl); @@ -741,16 +741,23 @@ static void DiagnoseHLSLRegisterAttribute(Sema &S, SourceLocation &ArgLoc, 1 && "only one resource analysis result should be expected"); - int regTypeNum = static_cast<int>(regType); + int RegTypeNum = static_cast<int>(RegType); // first, if "other" is set, emit an error if (Flags.Other) { - S.Diag(ArgLoc, diag::err_hlsl_binding_type_mismatch) << regTypeNum; + S.Diag(ArgLoc, diag::err_hlsl_binding_type_mismatch) << RegTypeNum; + return; + } + + // Space argument cannot be specified for global constants + if ((Flags.DefaultGlobals || Flags.UDT) && + !isDeclaredWithinCOrTBuffer(TheDecl) && SpecifiedSpace) { + S.Diag(ArgLoc, diag::err_hlsl_space_on_global_constant); return; } // next, if multiple register annotations exist, check that none conflict. - ValidateMultipleRegisterAnnotations(S, TheDecl, regType); + ValidateMultipleRegisterAnnotations(S, TheDecl, RegType); // next, if resource is set, make sure the register type in the register // annotation is compatible with the variable's resource type. @@ -782,9 +789,9 @@ static void DiagnoseHLSLRegisterAttribute(Sema &S, SourceLocation &ArgLoc, RegisterType ExpectedRegisterType = ExpectedRegisterTypesForResourceClass[(int)DeclResourceClass]; - if (regType != ExpectedRegisterType) { + if (RegType != ExpectedRegisterType) { S.Diag(TheDecl->getLocation(), diag::err_hlsl_binding_type_mismatch) - << regTypeNum; + << RegTypeNum; } return; } @@ -792,20 +799,17 @@ static void DiagnoseHLSLRegisterAttribute(Sema &S, SourceLocation &ArgLoc, // next, handle diagnostics for when the "basic" flag is set if (Flags.Basic) { if (Flags.DefaultGlobals) { - if (regType == RegisterType::CBuffer) + if (RegType == RegisterType::CBuffer) S.Diag(ArgLoc, diag::warn_hlsl_deprecated_register_type_b); - else if (regType != RegisterType::C) - S.Diag(ArgLoc, diag::err_hlsl_binding_type_mismatch) << regTypeNum; - // Space argument cannot be specified for global constants - if (AL.getNumArgs() == 2) - S.Diag(ArgLoc, diag::err_hlsl_space_on_global_constant); + else if (RegType != RegisterType::C) + S.Diag(ArgLoc, diag::err_hlsl_binding_type_mismatch) << RegTypeNum; return; } - if (regType == RegisterType::C) + if (RegType == RegisterType::C) S.Diag(ArgLoc, diag::warn_hlsl_register_type_c_packoffset); else - S.Diag(ArgLoc, diag::err_hlsl_binding_type_mismatch) << regTypeNum; + S.Diag(ArgLoc, diag::err_hlsl_binding_type_mismatch) << RegTypeNum; return; } @@ -814,19 +818,13 @@ static void DiagnoseHLSLRegisterAttribute(Sema &S, SourceLocation &ArgLoc, if (Flags.UDT) { const bool ExpectedRegisterTypesForUDT[] = { Flags.SRV, Flags.UAV, Flags.CBV, Flags.Sampler, Flags.ContainsNumeric}; - assert((size_t)regTypeNum < std::size(ExpectedRegisterTypesForUDT) && + assert((size_t)RegTypeNum < std::size(ExpectedRegisterTypesForUDT) && "regType has unexpected value"); - if (!ExpectedRegisterTypesForUDT[regTypeNum]) + if (!ExpectedRegisterTypesForUDT[RegTypeNum]) S.Diag(TheDecl->getLocation(), diag::warn_hlsl_user_defined_type_missing_member) - << regTypeNum; - // Space argument cannot be specified for global constants - if (!isDeclaredWithinCOrTBuffer(TheDecl)) { - if (AL.getNumArgs() == 2) - S.Diag(ArgLoc, diag::err_hlsl_space_on_global_constant); - return; - } + << RegTypeNum; } } @@ -851,7 +849,9 @@ void SemaHLSL::handleResourceBindingAttr(Decl *TheDecl, const ParsedAttr &AL) { SourceLocation ArgLoc = Loc->Loc; SourceLocation SpaceArgLoc; + bool SpecifiedSpace = false; if (AL.getNumArgs() == 2) { + SpecifiedSpace = true; Slot = Str; if (!AL.isArgIdent(1)) { Diag(AL.getLoc(), diag::err_attribute_argument_type) @@ -899,7 +899,8 @@ void SemaHLSL::handleResourceBindingAttr(Decl *TheDecl, const ParsedAttr &AL) { return; } - DiagnoseHLSLRegisterAttribute(SemaRef, ArgLoc, TheDecl, regType, AL); + DiagnoseHLSLRegisterAttribute(SemaRef, ArgLoc, TheDecl, regType, + SpecifiedSpace); HLSLResourceBindingAttr *NewAttr = HLSLResourceBindingAttr::Create(getASTContext(), Slot, Space, AL); diff --git a/clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl b/clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl index 892839384302ec..bd97b61e5d6e00 100644 --- a/clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl +++ b/clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl @@ -1,5 +1,24 @@ // RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -o - -fsyntax-only %s -verify +// valid +cbuffer cbuf { + RWBuffer<int> r : register(u0, space0); +} + +cbuffer cbuf2 { + struct x { + // expected-error@+1 {{'register' attribute only applies to cbuffer/tbuffer and external global variables}} + RWBuffer<int> E : register(u2, space3); + }; +} + +struct MyStruct { + RWBuffer<int> E; +}; + +cbuffer cbuf3 { + MyStruct E : register(u2, space3); +} // expected-error@+1 {{invalid space specifier 's2' used; expected 'space' followed by an integer, like space1}} cbuffer a : register(b0, s2) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits