[clang] 905de9b - [HLSL] Add testing for space parameter on global constants (#106782)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Sep 17 14:57:45 PDT 2024
Author: Joshua Batista
Date: 2024-09-17T14:57:41-07:00
New Revision: 905de9b0fe06d960e7f60175e6c96b955f334a66
URL: https://github.com/llvm/llvm-project/commit/905de9b0fe06d960e7f60175e6c96b955f334a66
DIFF: https://github.com/llvm/llvm-project/commit/905de9b0fe06d960e7f60175e6c96b955f334a66.diff
LOG: [HLSL] Add testing for space parameter on global constants (#106782)
The space parameter in the register binding annotation may not be used
for global constants. There was previously no diagnostic emitted when
this case occurred. This PR adds a diagnostic when this case occurs, and
tests these cases.
A new file was made to specifically test the space parameter, so some
cases in `\clang\test\SemaHLSL\resource_binding_attr_error.hlsl` were
moved over to this new file.
Fixes #104521
Added:
clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl
Modified:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaHLSL.cpp
clang/test/SemaHLSL/resource_binding_attr_error.hlsl
clang/test/SemaHLSL/resource_binding_attr_error_basic.hlsl
Removed:
################################################################################
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index e8b64f3c5a0187..bfda5b521c8fd2 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12381,6 +12381,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 67792be994fa89..a303f211501348 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -969,7 +969,8 @@ static void ValidateMultipleRegisterAnnotations(Sema &S, Decl *TheDecl,
}
static void DiagnoseHLSLRegisterAttribute(Sema &S, SourceLocation &ArgLoc,
- Decl *TheDecl, RegisterType regType) {
+ Decl *TheDecl, RegisterType RegType,
+ const bool SpecifiedSpace) {
// exactly one of these two types should be set
assert(((isa<VarDecl>(TheDecl) && !isa<HLSLBufferDecl>(TheDecl)) ||
@@ -982,42 +983,46 @@ 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;
}
// 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.
if (Flags.Resource) {
- RegisterType expRegType = getRegisterType(Flags.ResourceClass.value());
- if (regType != expRegType) {
+ RegisterType ExpRegType = getRegisterType(Flags.ResourceClass.value());
+ if (RegType != ExpRegType) {
S.Diag(TheDecl->getLocation(), diag::err_hlsl_binding_type_mismatch)
- << regTypeNum;
+ << RegTypeNum;
}
+
return;
}
// next, handle diagnostics for when the "basic" flag is set
if (Flags.Basic) {
+ if (SpecifiedSpace && !isDeclaredWithinCOrTBuffer(TheDecl))
+ S.Diag(ArgLoc, diag::err_hlsl_space_on_global_constant);
+
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;
+ 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;
}
@@ -1026,15 +1031,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;
-
- return;
+ << RegTypeNum;
}
}
@@ -1059,7 +1062,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)
@@ -1107,7 +1112,8 @@ void SemaHLSL::handleResourceBindingAttr(Decl *TheDecl, const ParsedAttr &AL) {
return;
}
- DiagnoseHLSLRegisterAttribute(SemaRef, ArgLoc, TheDecl, regType);
+ DiagnoseHLSLRegisterAttribute(SemaRef, ArgLoc, TheDecl, regType,
+ SpecifiedSpace);
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 cb728dca838c3b..74aff79f0e37fb 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 at +1 {{invalid space specifier 's2' used; expected 'space' followed by an integer, like space1}}
-cbuffer c : register(b0, s2) {
-}
// expected-error at +1 {{register number should be an integer}}
-cbuffer d : register(bf, s2) {
-
-}
-// expected-error at +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 at +1 {{wrong argument format for hlsl attribute, use b2 instead}}
cbuffer C : register(b 2) {}
-// expected-error at +2 {{wrong argument format for hlsl attribute, use b2 instead}}
-// expected-error at +1 {{wrong argument format for hlsl attribute, use space3 instead}}
-cbuffer D : register(b 2, space 3) {}
+// expected-error at +1 {{wrong argument format for hlsl attribute, use b2 instead}}
+cbuffer D : register(b 2, space3) {}
// expected-error at +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 at +1 {{binding type 'u' only applies to UAV resources}}
-float b : register(u0, space1);
-
// expected-error at +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 at +1{{binding type 't' only applies to SRV resources}}
float f1 : register(t0);
-
-float f2 : register(c0);
+// expected-error at +1 {{binding type 'u' only applies to UAV resources}}
+float f2 : register(u0);
// expected-error at +1{{binding type 'b' only applies to constant buffers. The 'bool constant' binding type is no longer supported}}
float f3 : register(b9);
+// expected-error at +1 {{binding type 's' only applies to sampler state}}
+float f4 : register(s0);
+
// expected-error at +1{{binding type 'i' ignored. The 'integer constant' binding type is no longer supported}}
-float f4 : register(i9);
+float f5 : register(i9);
// expected-error at +1{{binding type 'x' is invalid}}
-float f5 : register(x9);
+float f6 : register(x9);
cbuffer g_cbuffer1 {
// expected-error at +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 at +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 at +1{{binding type 'b' only applies to constant buffer resources}}
- float f8 : register(b2);
+ float f9 : register(b2);
};
tbuffer g_tbuffer2 {
// expected-error at +1{{binding type 'i' ignored. The 'integer constant' binding type is no longer supported}}
- float f9 : register(i2);
+ float f10 : register(i2);
};
// expected-error at +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..70e64e6ca75280
--- /dev/null
+++ b/clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl
@@ -0,0 +1,62 @@
+// 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 {
+ // this test validates that no diagnostic is emitted on the space parameter, because
+ // this register annotation is not in the global scope.
+ // expected-error at +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 {
+ // valid
+ MyStruct E : register(u2, space3);
+}
+
+// valid
+MyStruct F : register(u3, space4);
+
+cbuffer cbuf4 {
+ // this test validates that no diagnostic is emitted on the space parameter, because
+ // this register annotation is not in the global scope.
+ // expected-error at +1 {{binding type 'u' only applies to UAV resources}}
+ float a : register(u2, space3);
+}
+
+// expected-error at +1 {{invalid space specifier 's2' used; expected 'space' followed by an integer, like space1}}
+cbuffer a : register(b0, s2) {
+
+}
+
+// expected-error at +1 {{invalid space specifier 'spaces' used; expected 'space' followed by an integer, like space1}}
+cbuffer b : register(b2, spaces) {
+
+}
+
+// expected-error at +1 {{wrong argument format for hlsl attribute, use space3 instead}}
+cbuffer c : register(b2, space 3) {}
+
+// expected-error at +1 {{register space cannot be specified on global constants}}
+int d : register(c2, space3);
+
+// expected-error at +1 {{register space cannot be specified on global constants}}
+int e : register(c2, space0);
+
+// expected-error at +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);
More information about the cfe-commits
mailing list