[clang] Implement resource binding type prefix mismatch errors (PR #87578)
Joshua Batista via cfe-commits
cfe-commits at lists.llvm.org
Thu Apr 4 14:05:11 PDT 2024
https://github.com/bob80905 updated https://github.com/llvm/llvm-project/pull/87578
>From 3960050439964fe3c0536696490b284a6c470cd1 Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Wed, 3 Apr 2024 13:15:59 -0700
Subject: [PATCH 1/2] implement binding type error for t/cbuffers and rwbuffers
---
.../clang/Basic/DiagnosticSemaKinds.td | 1 +
clang/lib/Sema/HLSLExternalSemaSource.cpp | 19 +++--
clang/lib/Sema/SemaDeclAttr.cpp | 73 ++++++++++++++++++-
.../resource_binding_attr_error_mismatch.hlsl | 65 +++++++++++++++++
4 files changed, 149 insertions(+), 9 deletions(-)
create mode 100644 clang/test/SemaHLSL/resource_binding_attr_error_mismatch.hlsl
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 51af81bf1f6fc5..9a0946276f80fb 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12149,6 +12149,7 @@ def err_hlsl_missing_semantic_annotation : Error<
def err_hlsl_init_priority_unsupported : Error<
"initializer priorities are not supported in HLSL">;
+def err_hlsl_mismatching_register_type_and_name: Error<"invalid register name prefix '%0' for register type '%1' (expected '%2')">;
def err_hlsl_unsupported_register_type : Error<"invalid resource class specifier '%0' used; expected 'b', 's', 't', or 'u'">;
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">;
diff --git a/clang/lib/Sema/HLSLExternalSemaSource.cpp b/clang/lib/Sema/HLSLExternalSemaSource.cpp
index 1a1febf7a35241..479689ec82dcee 100644
--- a/clang/lib/Sema/HLSLExternalSemaSource.cpp
+++ b/clang/lib/Sema/HLSLExternalSemaSource.cpp
@@ -119,8 +119,10 @@ struct BuiltinTypeDeclBuilder {
ResourceKind RK, bool IsROV) {
if (Record->isCompleteDefinition())
return *this;
- Record->addAttr(HLSLResourceAttr::CreateImplicit(Record->getASTContext(),
- RC, RK, IsROV));
+ HLSLResourceAttr *attr = HLSLResourceAttr::CreateImplicit(
+ Record->getASTContext(), RC, RK, IsROV);
+
+ Record->addAttr(attr);
return *this;
}
@@ -482,15 +484,18 @@ static BuiltinTypeDeclBuilder setupBufferType(CXXRecordDecl *Decl, Sema &S,
bool IsROV) {
return BuiltinTypeDeclBuilder(Decl)
.addHandleMember()
- .addDefaultHandleConstructor(S, RC)
- .annotateResourceClass(RC, RK, IsROV);
+ .addDefaultHandleConstructor(S, RC);
}
void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
CXXRecordDecl *Decl;
- Decl = BuiltinTypeDeclBuilder(*SemaPtr, HLSLNamespace, "RWBuffer")
- .addSimpleTemplateParams({"element_type"})
- .Record;
+ Decl =
+ BuiltinTypeDeclBuilder(*SemaPtr, HLSLNamespace, "RWBuffer")
+ .addSimpleTemplateParams({"element_type"})
+ .annotateResourceClass(ResourceClass::UAV, ResourceKind::TypedBuffer,
+ /*IsROV=*/false)
+ .Record;
+
onCompletion(Decl, [this](CXXRecordDecl *Decl) {
setupBufferType(Decl, *SemaPtr, ResourceClass::UAV,
ResourceKind::TypedBuffer, /*IsROV=*/false)
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index f25f3afd0f4af2..e720fe56c3ea17 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -7333,12 +7333,12 @@ static void handleHLSLResourceBindingAttr(Sema &S, Decl *D,
} else {
Slot = Str;
}
-
+ QualType Ty = ((clang::ValueDecl *)D)->getType();
// Validate.
if (!Slot.empty()) {
switch (Slot[0]) {
- case 'u':
case 'b':
+ case 'u':
case 's':
case 't':
break;
@@ -7367,6 +7367,75 @@ static void handleHLSLResourceBindingAttr(Sema &S, Decl *D,
return;
}
+ VarDecl *VD = dyn_cast<VarDecl>(D);
+ HLSLBufferDecl *BD = dyn_cast<HLSLBufferDecl>(D);
+
+ if (VD || BD) {
+ llvm::hlsl::ResourceClass RC;
+ std::string varTy = "";
+ if (VD) {
+
+ const Type *Ty = VD->getType()->getPointeeOrArrayElementType();
+ if (!Ty)
+ return;
+ QualType t = ((ElaboratedType *)Ty)->getNamedType();
+ const CXXRecordDecl *RD = Ty->getAsCXXRecordDecl();
+ if (!RD)
+ return;
+
+ if (auto TDecl = dyn_cast<ClassTemplateSpecializationDecl>(RD))
+ RD = TDecl->getSpecializedTemplate()->getTemplatedDecl();
+ RD = RD->getCanonicalDecl();
+ const auto *Attr = RD->getAttr<HLSLResourceAttr>();
+ if (!Attr)
+ return;
+
+ RC = Attr->getResourceClass();
+ varTy = RD->getNameAsString();
+ } else {
+ if (BD->isCBuffer()) {
+ RC = llvm::hlsl::ResourceClass::CBuffer;
+ varTy = "cbuffer";
+ } else {
+ RC = llvm::hlsl::ResourceClass::CBuffer;
+ varTy = "tbuffer";
+ }
+ }
+ switch (RC) {
+ case llvm::hlsl::ResourceClass::SRV: {
+ if (Slot.substr(0, 1) != "t")
+ S.Diag(ArgLoc, diag::err_hlsl_mismatching_register_type_and_name)
+ << Slot.substr(0, 1) << varTy << "t";
+ break;
+ }
+ case llvm::hlsl::ResourceClass::UAV: {
+ if (Slot.substr(0, 1) != "u")
+ S.Diag(ArgLoc, diag::err_hlsl_mismatching_register_type_and_name)
+ << Slot.substr(0, 1) << varTy << "u";
+ break;
+ }
+ case llvm::hlsl::ResourceClass::CBuffer: {
+ if (Slot.substr(0, 1) != "b")
+ S.Diag(ArgLoc, diag::err_hlsl_mismatching_register_type_and_name)
+ << Slot.substr(0, 1) << varTy << "b";
+ break;
+ }
+ case llvm::hlsl::ResourceClass::Sampler: {
+ if (Slot.substr(0, 1) != "s")
+ S.Diag(ArgLoc, diag::err_hlsl_mismatching_register_type_and_name)
+ << Slot.substr(0, 1) << varTy << "s";
+ break;
+ }
+ case llvm::hlsl::ResourceClass::Invalid: {
+ llvm_unreachable("Resource class should be valid.");
+ break;
+ }
+
+ default:
+ break;
+ }
+ }
+
// FIXME: check reg type match decl. Issue
// https://github.com/llvm/llvm-project/issues/57886.
HLSLResourceBindingAttr *NewAttr =
diff --git a/clang/test/SemaHLSL/resource_binding_attr_error_mismatch.hlsl b/clang/test/SemaHLSL/resource_binding_attr_error_mismatch.hlsl
new file mode 100644
index 00000000000000..76dcbd201cbd19
--- /dev/null
+++ b/clang/test/SemaHLSL/resource_binding_attr_error_mismatch.hlsl
@@ -0,0 +1,65 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -o - -fsyntax-only %s -verify
+
+
+// expected-error at +1 {{invalid register name prefix 'b' for register type 'RWBuffer' (expected 'u')}}
+RWBuffer<int> a : register(b2, space1);
+
+// expected-error at +1 {{invalid register name prefix 't' for register type 'RWBuffer' (expected 'u')}}
+RWBuffer<int> b : register(t2, space1);
+
+// NOT YET IMPLEMENTED : {{invalid register name prefix 'u' for register type 'Texture1D' (expected 't')}}
+// NOT YET IMPLEMENTED Texture1D<float> tex : register(u3);
+
+// NOT YET IMPLEMENTED : {{invalid register name prefix 's' for register type 'Texture2D' (expected 't')}}
+// NOT YET IMPLEMENTED Texture2D<float> Texture : register(s0);
+
+// NOT YET IMPLEMENTED : {{invalid register name prefix 'u' for register type 'Texture2DMS' (expected 't')}}
+// NOT YET IMPLEMENTED Texture2DMS<float4, 4> T2DMS_t2 : register(u2)
+
+// NOT YET IMPLEMENTED : {{invalid register name prefix 't' for register type 'RWTexture3D' (expected 'u')}}
+// NOT YET IMPLEMENTED RWTexture3D<float4> RWT3D_u1 : register(t1)
+
+// NOT YET IMPLEMENTED : {{invalid register name prefix 'b' for register type 'Texture2DMS' (expected 't' or 's')}}
+// NOT YET IMPLEMENTED TextureCube TCube_b2 : register(B2);
+
+// NOT YET IMPLEMENTED : {{invalid register name prefix 'b' for register type 'Texture2DMS' (expected 't')}}
+// NOT YET IMPLEMENTED TextureCubeArray TCubeArray_t2 : register(b2);
+
+// NOT YET IMPLEMENTED : {{invalid register name prefix 'b' for register type 'Texture2DMS' (expected 't' or 's')}}
+// NOT YET IMPLEMENTED Texture1DArray T1DArray_t2 : register(b2);
+
+// NOT YET IMPLEMENTED : {{invalid register name prefix 'u' for register type 'Texture2DMS' (expected 't' or 's')}}
+// NOT YET IMPLEMENTED Texture2DArray T2DArray_b2 : register(B2);
+
+// NOT YET IMPLEMENTED : {{invalid register name prefix 'u' for register type 'Texture2DMS' (expected 'b' or 'c' or 'i')}}
+// NOT YET IMPLEMENTED Texture2DMSArray<float4> msTextureArray : register(t2, space2);
+
+// NOT YET IMPLEMENTED : {{invalid register name prefix 'u' for register type 'TCubeArray_f2' (expected 't' or 's')}}
+// NOT YET IMPLEMENTED TextureCubeArray TCubeArray_f2 : register(u2);
+
+// NOT YET IMPLEMENTED : {{invalid register name prefix 'u' for register type 'TypedBuffer' (expected 't')}}
+// NOT YET IMPLEMENTED TypedBuffer tbuf : register(u2);
+
+// NOT YET IMPLEMENTED : {{invalid register name prefix 'u' for register type 'RawBuffer' (expected 't')}}
+// NOT YET IMPLEMENTED RawBuffer rbuf : register(u2);
+
+// NOT YET IMPLEMENTED : {{invalid register name prefix 't' for register type 'StructuredBuffer' (expected 'u')}}
+// NOT YET IMPLEMENTED StructuredBuffer ROVStructuredBuff_t2 : register(T2);
+
+// expected-error at +1 {{invalid register name prefix 's' for register type 'cbuffer' (expected 'b')}}
+cbuffer f : register(s2, space1) {}
+
+// NOT YET IMPLEMENTED : {{invalid register name prefix 't' for register type 'Sampler' (expected 's')}}
+// Can this type just be Sampler instead of SamplerState?
+// NOT YET IMPLEMENTED SamplerState MySampler : register(t3, space1);
+
+// expected-error at +1 {{invalid register name prefix 's' for register type 'tbuffer' (expected 'b')}}
+tbuffer f : register(s2, space1) {}
+
+// NOT YET IMPLEMENTED : RTAccelerationStructure doesn't have any example tests in DXC
+
+// NOT YET IMPLEMENTED : {{invalid register name prefix 'u' for register type 'FeedbackTexture2D' (expected 'b' or 'c' or 'i')}}
+// NOT YET IMPLEMENTED FeedbackTexture2D<float> FBTex2D[3][] : register(u0, space26);
+
+// NOT YET IMPLEMENTED : {{invalid register name prefix 'u' for register type 'FeedbackTexture2DArray' (expected 'b' or 'c' or 'i')}}
+// NOT YET IMPLEMENTED FeedbackTexture2DArray<float> FBTex2DArr[3][2][] : register(u0, space27);
>From c307d0e2950976b1862db2a6d3d0005913da4f55 Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Thu, 4 Apr 2024 14:04:58 -0700
Subject: [PATCH 2/2] adjust switch statement, add empty string test cases
---
clang/lib/Sema/SemaDeclAttr.cpp | 15 +++++----------
.../resource_binding_attr_error_mismatch.hlsl | 8 ++++++++
2 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index e720fe56c3ea17..a2b60efbaf5383 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -7403,41 +7403,36 @@ static void handleHLSLResourceBindingAttr(Sema &S, Decl *D,
}
switch (RC) {
case llvm::hlsl::ResourceClass::SRV: {
- if (Slot.substr(0, 1) != "t")
+ if (Slot[0] != 't')
S.Diag(ArgLoc, diag::err_hlsl_mismatching_register_type_and_name)
<< Slot.substr(0, 1) << varTy << "t";
break;
}
case llvm::hlsl::ResourceClass::UAV: {
- if (Slot.substr(0, 1) != "u")
+ if (Slot[0] != 'u')
S.Diag(ArgLoc, diag::err_hlsl_mismatching_register_type_and_name)
<< Slot.substr(0, 1) << varTy << "u";
break;
}
case llvm::hlsl::ResourceClass::CBuffer: {
- if (Slot.substr(0, 1) != "b")
+ if (Slot[0] != 'b')
S.Diag(ArgLoc, diag::err_hlsl_mismatching_register_type_and_name)
<< Slot.substr(0, 1) << varTy << "b";
break;
}
case llvm::hlsl::ResourceClass::Sampler: {
- if (Slot.substr(0, 1) != "s")
+ if (Slot[0] != 's')
S.Diag(ArgLoc, diag::err_hlsl_mismatching_register_type_and_name)
<< Slot.substr(0, 1) << varTy << "s";
break;
}
- case llvm::hlsl::ResourceClass::Invalid: {
+ default: {
llvm_unreachable("Resource class should be valid.");
break;
}
-
- default:
- break;
}
}
- // FIXME: check reg type match decl. Issue
- // https://github.com/llvm/llvm-project/issues/57886.
HLSLResourceBindingAttr *NewAttr =
HLSLResourceBindingAttr::Create(S.getASTContext(), Slot, Space, AL);
if (NewAttr)
diff --git a/clang/test/SemaHLSL/resource_binding_attr_error_mismatch.hlsl b/clang/test/SemaHLSL/resource_binding_attr_error_mismatch.hlsl
index 76dcbd201cbd19..3bce7a0c290f1a 100644
--- a/clang/test/SemaHLSL/resource_binding_attr_error_mismatch.hlsl
+++ b/clang/test/SemaHLSL/resource_binding_attr_error_mismatch.hlsl
@@ -63,3 +63,11 @@ tbuffer f : register(s2, space1) {}
// NOT YET IMPLEMENTED : {{invalid register name prefix 'u' for register type 'FeedbackTexture2DArray' (expected 'b' or 'c' or 'i')}}
// NOT YET IMPLEMENTED FeedbackTexture2DArray<float> FBTex2DArr[3][2][] : register(u0, space27);
+
+
+// empty binding prefix cases:
+// expected-error at +1 {{expected identifier}}
+RWBuffer<int> c: register();
+
+// expected-error at +1 {{expected identifier}}
+RWBuffer<int> d: register("");
More information about the cfe-commits
mailing list