[clang] [llvm] Implement resource binding type prefix mismatch errors (PR #87578)
Joshua Batista via cfe-commits
cfe-commits at lists.llvm.org
Thu Apr 25 09:56:48 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 01/11] 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 02/11] 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("");
>From 2cde16b4a4a49fecce1bad03b590ba51f2d1fa10 Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Thu, 4 Apr 2024 14:25:36 -0700
Subject: [PATCH 03/11] use invalid case instead of default
---
clang/lib/Sema/SemaDeclAttr.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index a2b60efbaf5383..01224be123fc58 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -7426,7 +7426,7 @@ static void handleHLSLResourceBindingAttr(Sema &S, Decl *D,
<< Slot.substr(0, 1) << varTy << "s";
break;
}
- default: {
+ case llvm::hlsl::ResourceClass::Invalid: {
llvm_unreachable("Resource class should be valid.");
break;
}
>From c77b3ee1856cf668eb6d430e91abe1c2b6e8ac78 Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Wed, 10 Apr 2024 18:11:47 -0700
Subject: [PATCH 04/11] address part of feedback
---
clang/lib/Sema/HLSLExternalSemaSource.cpp | 6 ++----
clang/lib/Sema/SemaDeclAttr.cpp | 8 ++++----
2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/clang/lib/Sema/HLSLExternalSemaSource.cpp b/clang/lib/Sema/HLSLExternalSemaSource.cpp
index 479689ec82dcee..fb28fd33d09fd0 100644
--- a/clang/lib/Sema/HLSLExternalSemaSource.cpp
+++ b/clang/lib/Sema/HLSLExternalSemaSource.cpp
@@ -119,10 +119,8 @@ struct BuiltinTypeDeclBuilder {
ResourceKind RK, bool IsROV) {
if (Record->isCompleteDefinition())
return *this;
- HLSLResourceAttr *attr = HLSLResourceAttr::CreateImplicit(
- Record->getASTContext(), RC, RK, IsROV);
-
- Record->addAttr(attr);
+ Record->addAttr(HLSLResourceAttr::CreateImplicit(Record->getASTContext(),
+ RC, RK, IsROV));
return *this;
}
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 01224be123fc58..596818f43f4723 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -7333,7 +7333,7 @@ static void handleHLSLResourceBindingAttr(Sema &S, Decl *D,
} else {
Slot = Str;
}
- QualType Ty = ((clang::ValueDecl *)D)->getType();
+
// Validate.
if (!Slot.empty()) {
switch (Slot[0]) {
@@ -7372,13 +7372,13 @@ static void handleHLSLResourceBindingAttr(Sema &S, Decl *D,
if (VD || BD) {
llvm::hlsl::ResourceClass RC;
- std::string varTy = "";
+ StringRef varTy = "";
if (VD) {
const Type *Ty = VD->getType()->getPointeeOrArrayElementType();
if (!Ty)
return;
- QualType t = ((ElaboratedType *)Ty)->getNamedType();
+ QualType t = cast<ElaboratedType>(Ty)->getNamedType();
const CXXRecordDecl *RD = Ty->getAsCXXRecordDecl();
if (!RD)
return;
@@ -7391,7 +7391,7 @@ static void handleHLSLResourceBindingAttr(Sema &S, Decl *D,
return;
RC = Attr->getResourceClass();
- varTy = RD->getNameAsString();
+ varTy = RD->getName();
} else {
if (BD->isCBuffer()) {
RC = llvm::hlsl::ResourceClass::CBuffer;
>From 110eef6c6eef34d9266827067825905362d2f44a Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Wed, 17 Apr 2024 19:45:14 -0700
Subject: [PATCH 05/11] address rest of feedback, rename variables, create new
function
---
.../clang/Basic/DiagnosticSemaKinds.td | 2 +-
clang/lib/Sema/HLSLExternalSemaSource.cpp | 11 +-
clang/lib/Sema/SemaDeclAttr.cpp | 143 ++++++++++--------
.../test/AST/HLSL/resource_binding_attr.hlsl | 4 +-
clang/test/CodeGenHLSL/cbuf.hlsl | 2 +-
.../SemaHLSL/resource_binding_attr_error.hlsl | 2 +-
.../resource_binding_attr_error_mismatch.hlsl | 11 +-
7 files changed, 93 insertions(+), 82 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 9a0946276f80fb..b235d2ab858c07 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12149,7 +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_mismatching_register_type_and_name: Error<"invalid register name prefix '%0' for register type '%1' (expected %select{'t'|'u'|'b'|'s'}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 fb28fd33d09fd0..f208cfc0e1c058 100644
--- a/clang/lib/Sema/HLSLExternalSemaSource.cpp
+++ b/clang/lib/Sema/HLSLExternalSemaSource.cpp
@@ -478,8 +478,7 @@ void HLSLExternalSemaSource::defineTrivialHLSLTypes() {
/// Set up common members and attributes for buffer types
static BuiltinTypeDeclBuilder setupBufferType(CXXRecordDecl *Decl, Sema &S,
- ResourceClass RC, ResourceKind RK,
- bool IsROV) {
+ ResourceClass RC) {
return BuiltinTypeDeclBuilder(Decl)
.addHandleMember()
.addDefaultHandleConstructor(S, RC);
@@ -495,8 +494,7 @@ void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
.Record;
onCompletion(Decl, [this](CXXRecordDecl *Decl) {
- setupBufferType(Decl, *SemaPtr, ResourceClass::UAV,
- ResourceKind::TypedBuffer, /*IsROV=*/false)
+ setupBufferType(Decl, *SemaPtr, ResourceClass::UAV)
.addArraySubscriptOperators()
.completeDefinition();
});
@@ -504,10 +502,11 @@ void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
Decl =
BuiltinTypeDeclBuilder(*SemaPtr, HLSLNamespace, "RasterizerOrderedBuffer")
.addSimpleTemplateParams({"element_type"})
+ .annotateResourceClass(ResourceClass::UAV, ResourceKind::TypedBuffer,
+ /*IsROV=*/true)
.Record;
onCompletion(Decl, [this](CXXRecordDecl *Decl) {
- setupBufferType(Decl, *SemaPtr, ResourceClass::UAV,
- ResourceKind::TypedBuffer, /*IsROV=*/true)
+ setupBufferType(Decl, *SemaPtr, ResourceClass::UAV)
.addArraySubscriptOperators()
.completeDefinition();
});
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 596818f43f4723..653e827f7487f1 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -7303,6 +7303,81 @@ Sema::mergeHLSLShaderAttr(Decl *D, const AttributeCommonInfo &AL,
return HLSLShaderAttr::Create(Context, ShaderType, AL);
}
+static void DiagnoseHLSLResourceRegType(Sema &S, SourceLocation &ArgLoc,
+ Decl *D, StringRef &Slot) {
+ // Samplers, UAVs, and SRVs are VarDecl types
+ VarDecl *SamplerUAVOrSRV = dyn_cast<VarDecl>(D);
+ // Cbuffers and Tbuffers are HLSLBufferDecl types
+ HLSLBufferDecl *CBufferOrTBuffer = dyn_cast<HLSLBufferDecl>(D);
+ if (!SamplerUAVOrSRV && !CBufferOrTBuffer)
+ return;
+
+ llvm::hlsl::ResourceClass DeclResourceClass;
+ StringRef varTy = "";
+ if (SamplerUAVOrSRV) {
+ const Type *Ty = SamplerUAVOrSRV->getType()->getPointeeOrArrayElementType();
+ if (!Ty)
+ llvm_unreachable("Resource class should have an element type.");
+
+ const CXXRecordDecl *TheRecordDecl = Ty->getAsCXXRecordDecl();
+ if (!TheRecordDecl)
+ llvm_unreachable(
+ "Resource class should have a resource type declaration.");
+
+ if (auto TDecl = dyn_cast<ClassTemplateSpecializationDecl>(TheRecordDecl))
+ TheRecordDecl = TDecl->getSpecializedTemplate()->getTemplatedDecl();
+ TheRecordDecl = TheRecordDecl->getCanonicalDecl();
+ const auto *Attr = TheRecordDecl->getAttr<HLSLResourceAttr>();
+ if (!Attr)
+ llvm_unreachable("Resource class should have a resource attribute.");
+
+ DeclResourceClass = Attr->getResourceClass();
+ varTy = TheRecordDecl->getName();
+ } else {
+ if (CBufferOrTBuffer->isCBuffer()) {
+ DeclResourceClass = llvm::hlsl::ResourceClass::CBuffer;
+ varTy = "cbuffer";
+ } else {
+ DeclResourceClass = llvm::hlsl::ResourceClass::CBuffer;
+ varTy = "tbuffer";
+ }
+ }
+ switch (DeclResourceClass) {
+ case llvm::hlsl::ResourceClass::SRV: {
+ if (Slot[0] != 't')
+ S.Diag(ArgLoc, diag::err_hlsl_mismatching_register_type_and_name)
+ << Slot.substr(0, 1) << varTy
+ << (unsigned)llvm::hlsl::ResourceClass::SRV;
+ break;
+ }
+ case llvm::hlsl::ResourceClass::UAV: {
+ if (Slot[0] != 'u')
+ S.Diag(ArgLoc, diag::err_hlsl_mismatching_register_type_and_name)
+ << Slot.substr(0, 1) << varTy
+ << (unsigned)llvm::hlsl::ResourceClass::UAV;
+ break;
+ }
+ case llvm::hlsl::ResourceClass::CBuffer: {
+ if (Slot[0] != 'b')
+ S.Diag(ArgLoc, diag::err_hlsl_mismatching_register_type_and_name)
+ << Slot.substr(0, 1) << varTy
+ << (unsigned)llvm::hlsl::ResourceClass::CBuffer;
+ break;
+ }
+ case llvm::hlsl::ResourceClass::Sampler: {
+ if (Slot[0] != 's')
+ S.Diag(ArgLoc, diag::err_hlsl_mismatching_register_type_and_name)
+ << Slot.substr(0, 1) << varTy
+ << (unsigned)llvm::hlsl::ResourceClass::Sampler;
+ break;
+ }
+ case llvm::hlsl::ResourceClass::Invalid: {
+ llvm_unreachable("Resource class should be valid.");
+ break;
+ }
+ }
+}
+
static void handleHLSLResourceBindingAttr(Sema &S, Decl *D,
const ParsedAttr &AL) {
StringRef Space = "space0";
@@ -7337,8 +7412,8 @@ static void handleHLSLResourceBindingAttr(Sema &S, Decl *D,
// Validate.
if (!Slot.empty()) {
switch (Slot[0]) {
- case 'b':
case 'u':
+ case 'b':
case 's':
case 't':
break;
@@ -7367,71 +7442,7 @@ 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;
- StringRef varTy = "";
- if (VD) {
-
- const Type *Ty = VD->getType()->getPointeeOrArrayElementType();
- if (!Ty)
- return;
- QualType t = cast<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->getName();
- } 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[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[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[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[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: {
- llvm_unreachable("Resource class should be valid.");
- break;
- }
- }
- }
+ DiagnoseHLSLResourceRegType(S, ArgLoc, D, Slot);
HLSLResourceBindingAttr *NewAttr =
HLSLResourceBindingAttr::Create(S.getASTContext(), Slot, Space, AL);
diff --git a/clang/test/AST/HLSL/resource_binding_attr.hlsl b/clang/test/AST/HLSL/resource_binding_attr.hlsl
index 71900f2dbda550..696c563786277a 100644
--- a/clang/test/AST/HLSL/resource_binding_attr.hlsl
+++ b/clang/test/AST/HLSL/resource_binding_attr.hlsl
@@ -8,9 +8,9 @@ cbuffer CB : register(b3, space2) {
}
// CHECK:HLSLBufferDecl 0x[[TB:[0-9a-f]+]] {{.*}} line:13:9 tbuffer TB
-// CHECK-NEXT:HLSLResourceBindingAttr 0x{{[0-9a-f]+}} <col:14> "t2" "space1"
+// CHECK-NEXT:HLSLResourceBindingAttr 0x{{[0-9a-f]+}} <col:14> "b2" "space1"
// CHECK-NEXT:VarDecl 0x[[B:[0-9a-f]+]] {{.*}} col:9 used b 'float'
-tbuffer TB : register(t2, space1) {
+tbuffer TB : register(b2, space1) {
float b;
}
diff --git a/clang/test/CodeGenHLSL/cbuf.hlsl b/clang/test/CodeGenHLSL/cbuf.hlsl
index dc2a6aaa8f4335..a4e817890cd959 100644
--- a/clang/test/CodeGenHLSL/cbuf.hlsl
+++ b/clang/test/CodeGenHLSL/cbuf.hlsl
@@ -9,7 +9,7 @@ cbuffer A : register(b0, space2) {
}
// CHECK: @[[TB:.+]] = external constant { float, double }
-tbuffer A : register(t2, space1) {
+tbuffer A : register(b2, space1) {
float c;
double d;
}
diff --git a/clang/test/SemaHLSL/resource_binding_attr_error.hlsl b/clang/test/SemaHLSL/resource_binding_attr_error.hlsl
index 95774472aaea08..42c9e942bb1a56 100644
--- a/clang/test/SemaHLSL/resource_binding_attr_error.hlsl
+++ b/clang/test/SemaHLSL/resource_binding_attr_error.hlsl
@@ -45,7 +45,7 @@ void foo2() {
extern RWBuffer<float> U2 : register(u5);
}
// FIXME: expect-error once fix https://github.com/llvm/llvm-project/issues/57886.
-float b : register(u0, space1);
+// float b : register(u0, space1) {}
// expected-warning at +1 {{'register' attribute only applies to cbuffer/tbuffer and external global variables}}
void bar(RWBuffer<float> U : register(u3)) {
diff --git a/clang/test/SemaHLSL/resource_binding_attr_error_mismatch.hlsl b/clang/test/SemaHLSL/resource_binding_attr_error_mismatch.hlsl
index 3bce7a0c290f1a..8544a2a37949b9 100644
--- a/clang/test/SemaHLSL/resource_binding_attr_error_mismatch.hlsl
+++ b/clang/test/SemaHLSL/resource_binding_attr_error_mismatch.hlsl
@@ -1,11 +1,12 @@
// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -o - -fsyntax-only %s -verify
+// the below will cause an llvm unreachable, because RWBuffers don't have resource attributes yet
+// NOT YET IMPLEMENTED : {{invalid register name prefix 'b' for register type 'RWBuffer' (expected 'u')}}
+// NOT YET IMPLEMENTED RWBuffer<int> a : register(b2, space1);
-// 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);
+// the below will cause an llvm unreachable, because RWBuffers don't have resource attributes yet
+// NOT YET IMPLEMENTED : {{invalid register name prefix 't' for register type 'RWBuffer' (expected 'u')}}
+// NOT YET IMPLEMENTED 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);
>From 1a9ea6c1c4792c27f8da401e76ac21909517cee1 Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Thu, 18 Apr 2024 18:46:23 -0700
Subject: [PATCH 06/11] remove unneeded function
---
clang/lib/Sema/SemaDeclAttr.cpp | 13 -------------
1 file changed, 13 deletions(-)
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 7424f593f84110..f8ff4515d2a26b 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -7334,19 +7334,6 @@ static void handleHLSLShaderAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
D->addAttr(NewAttr);
}
-HLSLShaderAttr *
-Sema::mergeHLSLShaderAttr(Decl *D, const AttributeCommonInfo &AL,
- HLSLShaderAttr::ShaderType ShaderType) {
- if (HLSLShaderAttr *NT = D->getAttr<HLSLShaderAttr>()) {
- if (NT->getType() != ShaderType) {
- Diag(NT->getLocation(), diag::err_hlsl_attribute_param_mismatch) << AL;
- Diag(AL.getLoc(), diag::note_conflicting_attribute);
- }
- return nullptr;
- }
- return HLSLShaderAttr::Create(Context, ShaderType, AL);
-}
-
static void DiagnoseHLSLResourceRegType(Sema &S, SourceLocation &ArgLoc,
Decl *D, StringRef &Slot) {
// Samplers, UAVs, and SRVs are VarDecl types
>From f985ab69433f82610576a1e8b98a09077c78a48c Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Mon, 22 Apr 2024 10:30:09 -0700
Subject: [PATCH 07/11] format
---
clang/lib/Serialization/ASTWriter.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 2eb4adac53de2a..3aaad2381e36bf 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -5014,7 +5014,7 @@ void ASTWriter::WriteSpecialDeclRecords(Sema &SemaRef) {
if (!ModularCodegenDecls.empty())
Stream.EmitRecord(MODULAR_CODEGEN_DECLS, ModularCodegenDecls);
-
+
// Write the record containing tentative definitions.
RecordData TentativeDefinitions;
AddLazyVectorEmiitedDecls(*this, SemaRef.TentativeDefinitions,
@@ -5135,7 +5135,7 @@ void ASTWriter::WriteSpecialDeclRecords(Sema &SemaRef) {
}
if (!UndefinedButUsed.empty())
Stream.EmitRecord(UNDEFINED_BUT_USED, UndefinedButUsed);
-
+
// Write all delete-expressions that we would like to
// analyze later in AST.
RecordData DeleteExprsToAnalyze;
>From 3b2afe7c5282e16604d3ac05f6b25075cbfde4b8 Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Tue, 23 Apr 2024 16:00:22 -0700
Subject: [PATCH 08/11] address all of Justin and Xiang's feedback
---
.../clang/Basic/DiagnosticSemaKinds.td | 3 +-
clang/lib/Sema/HLSLExternalSemaSource.cpp | 8 ++--
clang/lib/Sema/SemaDeclAttr.cpp | 47 +++++++++++++------
.../test/AST/HLSL/resource_binding_attr.hlsl | 4 +-
clang/test/CodeGenHLSL/cbuf.hlsl | 2 +-
.../SemaHLSL/resource_binding_attr_error.hlsl | 4 +-
.../resource_binding_attr_error_mismatch.hlsl | 12 ++---
.../include/llvm/Frontend/HLSL/HLSLResource.h | 1 +
8 files changed, 51 insertions(+), 30 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 3654dccfea978c..de940625b44b48 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12154,7 +12154,8 @@ 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 %select{'t'|'u'|'b'|'s'}2)">;
+def err_hlsl_mismatching_register_resource_type_and_name: Error<"invalid register name prefix '%0' for register resource type '%1' (expected %select{'t'|'u'|'b'|'t'|'s'}2)">;
+def err_hlsl_mismatching_register_builtin_type_and_name: Error<"invalid register name prefix '%0' for '%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 f208cfc0e1c058..b9a6a5baefd254 100644
--- a/clang/lib/Sema/HLSLExternalSemaSource.cpp
+++ b/clang/lib/Sema/HLSLExternalSemaSource.cpp
@@ -477,8 +477,8 @@ void HLSLExternalSemaSource::defineTrivialHLSLTypes() {
}
/// Set up common members and attributes for buffer types
-static BuiltinTypeDeclBuilder setupBufferType(CXXRecordDecl *Decl, Sema &S,
- ResourceClass RC) {
+static BuiltinTypeDeclBuilder setupBufferHandle(CXXRecordDecl *Decl, Sema &S,
+ ResourceClass RC) {
return BuiltinTypeDeclBuilder(Decl)
.addHandleMember()
.addDefaultHandleConstructor(S, RC);
@@ -494,7 +494,7 @@ void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
.Record;
onCompletion(Decl, [this](CXXRecordDecl *Decl) {
- setupBufferType(Decl, *SemaPtr, ResourceClass::UAV)
+ setupBufferHandle(Decl, *SemaPtr, ResourceClass::UAV)
.addArraySubscriptOperators()
.completeDefinition();
});
@@ -506,7 +506,7 @@ void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
/*IsROV=*/true)
.Record;
onCompletion(Decl, [this](CXXRecordDecl *Decl) {
- setupBufferType(Decl, *SemaPtr, ResourceClass::UAV)
+ setupBufferHandle(Decl, *SemaPtr, ResourceClass::UAV)
.addArraySubscriptOperators()
.completeDefinition();
});
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index f8ff4515d2a26b..7095825c7d842a 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -7344,11 +7344,23 @@ static void DiagnoseHLSLResourceRegType(Sema &S, SourceLocation &ArgLoc,
return;
llvm::hlsl::ResourceClass DeclResourceClass;
- StringRef varTy = "";
+ StringRef VarTy = "";
if (SamplerUAVOrSRV) {
const Type *Ty = SamplerUAVOrSRV->getType()->getPointeeOrArrayElementType();
if (!Ty)
- llvm_unreachable("Resource class should have an element type.");
+ llvm_unreachable("Resource class must have an element type.");
+
+ if (const BuiltinType *BTy = dyn_cast<BuiltinType>(Ty)) {
+ QualType QT = SamplerUAVOrSRV->getType();
+ PrintingPolicy PP = S.getPrintingPolicy();
+ std::string typestr = QualType::getAsString(QT.split(), PP);
+
+ if (Slot[0] != 'b' && Slot[0] != 'c' && Slot[0] != 'i')
+ S.Diag(ArgLoc,
+ diag::err_hlsl_mismatching_register_builtin_type_and_name)
+ << Slot.substr(0, 1) << typestr << "'b, c, or i";
+ return;
+ }
const CXXRecordDecl *TheRecordDecl = Ty->getAsCXXRecordDecl();
if (!TheRecordDecl)
@@ -7363,42 +7375,49 @@ static void DiagnoseHLSLResourceRegType(Sema &S, SourceLocation &ArgLoc,
llvm_unreachable("Resource class should have a resource attribute.");
DeclResourceClass = Attr->getResourceClass();
- varTy = TheRecordDecl->getName();
+ VarTy = TheRecordDecl->getName();
} else {
if (CBufferOrTBuffer->isCBuffer()) {
DeclResourceClass = llvm::hlsl::ResourceClass::CBuffer;
- varTy = "cbuffer";
+ VarTy = "cbuffer";
} else {
- DeclResourceClass = llvm::hlsl::ResourceClass::CBuffer;
- varTy = "tbuffer";
+ DeclResourceClass = llvm::hlsl::ResourceClass::TBuffer;
+ VarTy = "tbuffer";
}
}
switch (DeclResourceClass) {
case llvm::hlsl::ResourceClass::SRV: {
if (Slot[0] != 't')
- S.Diag(ArgLoc, diag::err_hlsl_mismatching_register_type_and_name)
- << Slot.substr(0, 1) << varTy
+ S.Diag(ArgLoc, diag::err_hlsl_mismatching_register_resource_type_and_name)
+ << Slot.substr(0, 1) << VarTy
<< (unsigned)llvm::hlsl::ResourceClass::SRV;
break;
}
case llvm::hlsl::ResourceClass::UAV: {
if (Slot[0] != 'u')
- S.Diag(ArgLoc, diag::err_hlsl_mismatching_register_type_and_name)
- << Slot.substr(0, 1) << varTy
+ S.Diag(ArgLoc, diag::err_hlsl_mismatching_register_resource_type_and_name)
+ << Slot.substr(0, 1) << VarTy
<< (unsigned)llvm::hlsl::ResourceClass::UAV;
break;
}
case llvm::hlsl::ResourceClass::CBuffer: {
if (Slot[0] != 'b')
- S.Diag(ArgLoc, diag::err_hlsl_mismatching_register_type_and_name)
- << Slot.substr(0, 1) << varTy
+ S.Diag(ArgLoc, diag::err_hlsl_mismatching_register_resource_type_and_name)
+ << Slot.substr(0, 1) << VarTy
<< (unsigned)llvm::hlsl::ResourceClass::CBuffer;
break;
}
+ case llvm::hlsl::ResourceClass::TBuffer: {
+ if (Slot[0] != 't')
+ S.Diag(ArgLoc, diag::err_hlsl_mismatching_register_resource_type_and_name)
+ << Slot.substr(0, 1) << VarTy
+ << (unsigned)llvm::hlsl::ResourceClass::TBuffer;
+ break;
+ }
case llvm::hlsl::ResourceClass::Sampler: {
if (Slot[0] != 's')
- S.Diag(ArgLoc, diag::err_hlsl_mismatching_register_type_and_name)
- << Slot.substr(0, 1) << varTy
+ S.Diag(ArgLoc, diag::err_hlsl_mismatching_register_resource_type_and_name)
+ << Slot.substr(0, 1) << VarTy
<< (unsigned)llvm::hlsl::ResourceClass::Sampler;
break;
}
diff --git a/clang/test/AST/HLSL/resource_binding_attr.hlsl b/clang/test/AST/HLSL/resource_binding_attr.hlsl
index 696c563786277a..71900f2dbda550 100644
--- a/clang/test/AST/HLSL/resource_binding_attr.hlsl
+++ b/clang/test/AST/HLSL/resource_binding_attr.hlsl
@@ -8,9 +8,9 @@ cbuffer CB : register(b3, space2) {
}
// CHECK:HLSLBufferDecl 0x[[TB:[0-9a-f]+]] {{.*}} line:13:9 tbuffer TB
-// CHECK-NEXT:HLSLResourceBindingAttr 0x{{[0-9a-f]+}} <col:14> "b2" "space1"
+// CHECK-NEXT:HLSLResourceBindingAttr 0x{{[0-9a-f]+}} <col:14> "t2" "space1"
// CHECK-NEXT:VarDecl 0x[[B:[0-9a-f]+]] {{.*}} col:9 used b 'float'
-tbuffer TB : register(b2, space1) {
+tbuffer TB : register(t2, space1) {
float b;
}
diff --git a/clang/test/CodeGenHLSL/cbuf.hlsl b/clang/test/CodeGenHLSL/cbuf.hlsl
index a4e817890cd959..dc2a6aaa8f4335 100644
--- a/clang/test/CodeGenHLSL/cbuf.hlsl
+++ b/clang/test/CodeGenHLSL/cbuf.hlsl
@@ -9,7 +9,7 @@ cbuffer A : register(b0, space2) {
}
// CHECK: @[[TB:.+]] = external constant { float, double }
-tbuffer A : register(b2, space1) {
+tbuffer A : register(t2, space1) {
float c;
double d;
}
diff --git a/clang/test/SemaHLSL/resource_binding_attr_error.hlsl b/clang/test/SemaHLSL/resource_binding_attr_error.hlsl
index 42c9e942bb1a56..c2e09c116654f9 100644
--- a/clang/test/SemaHLSL/resource_binding_attr_error.hlsl
+++ b/clang/test/SemaHLSL/resource_binding_attr_error.hlsl
@@ -44,8 +44,8 @@ void foo2() {
// expected-warning at +1 {{'register' attribute only applies to cbuffer/tbuffer and external global variables}}
extern RWBuffer<float> U2 : register(u5);
}
-// FIXME: expect-error once fix https://github.com/llvm/llvm-project/issues/57886.
-// float b : register(u0, space1) {}
+// expected-error at +1 {{invalid register name prefix 'u' for 'float' (expected 'b, c, or i)}}
+float b : register(u0, space1);
// expected-warning at +1 {{'register' attribute only applies to cbuffer/tbuffer and external global variables}}
void bar(RWBuffer<float> U : register(u3)) {
diff --git a/clang/test/SemaHLSL/resource_binding_attr_error_mismatch.hlsl b/clang/test/SemaHLSL/resource_binding_attr_error_mismatch.hlsl
index 8544a2a37949b9..4bff910c0a7a4c 100644
--- a/clang/test/SemaHLSL/resource_binding_attr_error_mismatch.hlsl
+++ b/clang/test/SemaHLSL/resource_binding_attr_error_mismatch.hlsl
@@ -1,12 +1,12 @@
// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -o - -fsyntax-only %s -verify
// the below will cause an llvm unreachable, because RWBuffers don't have resource attributes yet
-// NOT YET IMPLEMENTED : {{invalid register name prefix 'b' for register type 'RWBuffer' (expected 'u')}}
-// NOT YET IMPLEMENTED RWBuffer<int> a : register(b2, space1);
+// expected-error at +1 {{invalid register name prefix 'b' for register resource type 'RWBuffer' (expected 'u')}}
+RWBuffer<int> a : register(b2, space1);
// the below will cause an llvm unreachable, because RWBuffers don't have resource attributes yet
-// NOT YET IMPLEMENTED : {{invalid register name prefix 't' for register type 'RWBuffer' (expected 'u')}}
-// NOT YET IMPLEMENTED RWBuffer<int> b : register(t2, space1);
+// expected-error at +1 {{invalid register name prefix 't' for register resource 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);
@@ -47,14 +47,14 @@
// 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')}}
+// expected-error at +1 {{invalid register name prefix 's' for register resource 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')}}
+// expected-error at +1 {{invalid register name prefix 's' for register resource type 'tbuffer' (expected 't')}}
tbuffer f : register(s2, space1) {}
// NOT YET IMPLEMENTED : RTAccelerationStructure doesn't have any example tests in DXC
diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLResource.h b/llvm/include/llvm/Frontend/HLSL/HLSLResource.h
index edfcbda0a3bb38..61ec920b76092f 100644
--- a/llvm/include/llvm/Frontend/HLSL/HLSLResource.h
+++ b/llvm/include/llvm/Frontend/HLSL/HLSLResource.h
@@ -25,6 +25,7 @@ enum class ResourceClass : uint8_t {
SRV = 0,
UAV,
CBuffer,
+ TBuffer,
Sampler,
Invalid,
NumClasses = Invalid,
>From dcaf3ad9012e5f2fd12219da2e0204470a41c7a9 Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Tue, 23 Apr 2024 16:59:44 -0700
Subject: [PATCH 09/11] address tex
---
clang/lib/Sema/SemaDeclAttr.cpp | 4 ++--
.../SemaHLSL/resource_binding_attr_error.hlsl | 2 +-
.../resource_binding_attr_error_mismatch.hlsl | 18 +++++++++---------
3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 7095825c7d842a..172c68c8dd7e4e 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -7355,10 +7355,10 @@ static void DiagnoseHLSLResourceRegType(Sema &S, SourceLocation &ArgLoc,
PrintingPolicy PP = S.getPrintingPolicy();
std::string typestr = QualType::getAsString(QT.split(), PP);
- if (Slot[0] != 'b' && Slot[0] != 'c' && Slot[0] != 'i')
+ if (Slot[0] != 't')
S.Diag(ArgLoc,
diag::err_hlsl_mismatching_register_builtin_type_and_name)
- << Slot.substr(0, 1) << typestr << "'b, c, or i";
+ << Slot.substr(0, 1) << typestr << "'t'";
return;
}
diff --git a/clang/test/SemaHLSL/resource_binding_attr_error.hlsl b/clang/test/SemaHLSL/resource_binding_attr_error.hlsl
index c2e09c116654f9..5d3b742eae36ff 100644
--- a/clang/test/SemaHLSL/resource_binding_attr_error.hlsl
+++ b/clang/test/SemaHLSL/resource_binding_attr_error.hlsl
@@ -44,7 +44,7 @@ void foo2() {
// expected-warning at +1 {{'register' attribute only applies to cbuffer/tbuffer and external global variables}}
extern RWBuffer<float> U2 : register(u5);
}
-// expected-error at +1 {{invalid register name prefix 'u' for 'float' (expected 'b, c, or i)}}
+// expected-error at +1 {{invalid register name prefix 'u' for 'float' (expected 't')}}
float b : register(u0, space1);
// expected-warning at +1 {{'register' attribute only applies to cbuffer/tbuffer and external global variables}}
diff --git a/clang/test/SemaHLSL/resource_binding_attr_error_mismatch.hlsl b/clang/test/SemaHLSL/resource_binding_attr_error_mismatch.hlsl
index 4bff910c0a7a4c..32bf3c510f2acd 100644
--- a/clang/test/SemaHLSL/resource_binding_attr_error_mismatch.hlsl
+++ b/clang/test/SemaHLSL/resource_binding_attr_error_mismatch.hlsl
@@ -20,22 +20,22 @@ RWBuffer<int> b : register(t2, space1);
// 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 'TextureCube' (expected 't')}}
+// NOT YET IMPLEMENTED TextureCube <float> t8 : register(b8);
-// NOT YET IMPLEMENTED : {{invalid register name prefix 'b' for register type 'Texture2DMS' (expected 't')}}
+// NOT YET IMPLEMENTED : {{invalid register name prefix 'b' for register type 'TextureCubeArray' (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 : {{invalid register name prefix 'b' for register type 'Texture1DArray' (expected 't')}}
// 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 : {{invalid register name prefix 'u' for register type 'Texture2DArray' (expected 't')}}
// 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 : {{invalid register name prefix 'u' for register type 'Texture2DMSArray' (expected 't')}}
// 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 : {{invalid register name prefix 'u' for register type 'TextureCubeArray' (expected 't')}}
// NOT YET IMPLEMENTED TextureCubeArray TCubeArray_f2 : register(u2);
// NOT YET IMPLEMENTED : {{invalid register name prefix 'u' for register type 'TypedBuffer' (expected 't')}}
@@ -59,10 +59,10 @@ 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 : {{invalid register name prefix 'u' for register type 'FeedbackTexture2D' (expected 't')}}
// 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 : {{invalid register name prefix 'u' for register type 'FeedbackTexture2DArray' (expected 't')}}
// NOT YET IMPLEMENTED FeedbackTexture2DArray<float> FBTex2DArr[3][2][] : register(u0, space27);
>From 1edee6ed24c1c4f5321e1352556bc2d4c1146c95 Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Wed, 24 Apr 2024 15:44:22 -0700
Subject: [PATCH 10/11] clean up switch statement, remove bad comments
---
clang/lib/Sema/SemaDeclAttr.cpp | 32 +++++++------------
.../resource_binding_attr_error_mismatch.hlsl | 3 +-
2 files changed, 13 insertions(+), 22 deletions(-)
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 172c68c8dd7e4e..9682e9c6694ced 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -7387,38 +7387,28 @@ static void DiagnoseHLSLResourceRegType(Sema &S, SourceLocation &ArgLoc,
}
switch (DeclResourceClass) {
case llvm::hlsl::ResourceClass::SRV: {
- if (Slot[0] != 't')
- S.Diag(ArgLoc, diag::err_hlsl_mismatching_register_resource_type_and_name)
- << Slot.substr(0, 1) << VarTy
- << (unsigned)llvm::hlsl::ResourceClass::SRV;
+ if (Slot[0] == 't')
+ return;
break;
}
case llvm::hlsl::ResourceClass::UAV: {
- if (Slot[0] != 'u')
- S.Diag(ArgLoc, diag::err_hlsl_mismatching_register_resource_type_and_name)
- << Slot.substr(0, 1) << VarTy
- << (unsigned)llvm::hlsl::ResourceClass::UAV;
+ if (Slot[0] == 'u')
+ return;
break;
}
case llvm::hlsl::ResourceClass::CBuffer: {
- if (Slot[0] != 'b')
- S.Diag(ArgLoc, diag::err_hlsl_mismatching_register_resource_type_and_name)
- << Slot.substr(0, 1) << VarTy
- << (unsigned)llvm::hlsl::ResourceClass::CBuffer;
+ if (Slot[0] == 'b')
+ return;
break;
}
case llvm::hlsl::ResourceClass::TBuffer: {
- if (Slot[0] != 't')
- S.Diag(ArgLoc, diag::err_hlsl_mismatching_register_resource_type_and_name)
- << Slot.substr(0, 1) << VarTy
- << (unsigned)llvm::hlsl::ResourceClass::TBuffer;
+ if (Slot[0] == 't')
+ return;
break;
}
case llvm::hlsl::ResourceClass::Sampler: {
- if (Slot[0] != 's')
- S.Diag(ArgLoc, diag::err_hlsl_mismatching_register_resource_type_and_name)
- << Slot.substr(0, 1) << VarTy
- << (unsigned)llvm::hlsl::ResourceClass::Sampler;
+ if (Slot[0] == 's')
+ return;
break;
}
case llvm::hlsl::ResourceClass::Invalid: {
@@ -7426,6 +7416,8 @@ static void DiagnoseHLSLResourceRegType(Sema &S, SourceLocation &ArgLoc,
break;
}
}
+ S.Diag(ArgLoc, diag::err_hlsl_mismatching_register_resource_type_and_name)
+ << Slot.substr(0, 1) << VarTy << (unsigned)DeclResourceClass;
}
static void handleHLSLResourceBindingAttr(Sema &S, Decl *D,
diff --git a/clang/test/SemaHLSL/resource_binding_attr_error_mismatch.hlsl b/clang/test/SemaHLSL/resource_binding_attr_error_mismatch.hlsl
index 32bf3c510f2acd..249a24d980ba5b 100644
--- a/clang/test/SemaHLSL/resource_binding_attr_error_mismatch.hlsl
+++ b/clang/test/SemaHLSL/resource_binding_attr_error_mismatch.hlsl
@@ -1,10 +1,9 @@
// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -o - -fsyntax-only %s -verify
-// the below will cause an llvm unreachable, because RWBuffers don't have resource attributes yet
+
// expected-error at +1 {{invalid register name prefix 'b' for register resource type 'RWBuffer' (expected 'u')}}
RWBuffer<int> a : register(b2, space1);
-// the below will cause an llvm unreachable, because RWBuffers don't have resource attributes yet
// expected-error at +1 {{invalid register name prefix 't' for register resource type 'RWBuffer' (expected 'u')}}
RWBuffer<int> b : register(t2, space1);
>From 9bdcea237858ca581681bf7b75d54af24376ce09 Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Thu, 25 Apr 2024 09:56:25 -0700
Subject: [PATCH 11/11] remove tbuffer as a resource class
---
clang/lib/Sema/SemaDeclAttr.cpp | 28 +++++++++----------
.../include/llvm/Frontend/HLSL/HLSLResource.h | 1 -
2 files changed, 14 insertions(+), 15 deletions(-)
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 9682e9c6694ced..095290caf14c0c 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -7377,13 +7377,7 @@ static void DiagnoseHLSLResourceRegType(Sema &S, SourceLocation &ArgLoc,
DeclResourceClass = Attr->getResourceClass();
VarTy = TheRecordDecl->getName();
} else {
- if (CBufferOrTBuffer->isCBuffer()) {
- DeclResourceClass = llvm::hlsl::ResourceClass::CBuffer;
- VarTy = "cbuffer";
- } else {
- DeclResourceClass = llvm::hlsl::ResourceClass::TBuffer;
- VarTy = "tbuffer";
- }
+ DeclResourceClass = llvm::hlsl::ResourceClass::CBuffer;
}
switch (DeclResourceClass) {
case llvm::hlsl::ResourceClass::SRV: {
@@ -7397,13 +7391,19 @@ static void DiagnoseHLSLResourceRegType(Sema &S, SourceLocation &ArgLoc,
break;
}
case llvm::hlsl::ResourceClass::CBuffer: {
- if (Slot[0] == 'b')
- return;
- break;
- }
- case llvm::hlsl::ResourceClass::TBuffer: {
- if (Slot[0] == 't')
- return;
+ // could be CBuffer or TBuffer
+ if (CBufferOrTBuffer->isCBuffer()) {
+ VarTy = "cbuffer";
+ if (Slot[0] == 'b')
+ return;
+ } else {
+ VarTy = "tbuffer";
+ // This isn't an SRV, but we need the diagnostic to emit
+ // the same binding prefix that would be expected on an SRV.
+ DeclResourceClass = llvm::hlsl::ResourceClass::SRV;
+ if (Slot[0] == 't')
+ return;
+ }
break;
}
case llvm::hlsl::ResourceClass::Sampler: {
diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLResource.h b/llvm/include/llvm/Frontend/HLSL/HLSLResource.h
index 61ec920b76092f..edfcbda0a3bb38 100644
--- a/llvm/include/llvm/Frontend/HLSL/HLSLResource.h
+++ b/llvm/include/llvm/Frontend/HLSL/HLSLResource.h
@@ -25,7 +25,6 @@ enum class ResourceClass : uint8_t {
SRV = 0,
UAV,
CBuffer,
- TBuffer,
Sampler,
Invalid,
NumClasses = Invalid,
More information about the cfe-commits
mailing list