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 <jbati...@microsoft.com> Date: Wed, 3 Apr 2024 13:15:59 -0700 Subject: [PATCH 01/10] 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@+1 {{invalid register name prefix 'b' for register type 'RWBuffer' (expected 'u')}} +RWBuffer<int> a : register(b2, space1); + +// expected-error@+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@+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@+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 <jbati...@microsoft.com> Date: Thu, 4 Apr 2024 14:04:58 -0700 Subject: [PATCH 02/10] 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@+1 {{expected identifier}} +RWBuffer<int> c: register(); + +// expected-error@+1 {{expected identifier}} +RWBuffer<int> d: register(""); >From 2cde16b4a4a49fecce1bad03b590ba51f2d1fa10 Mon Sep 17 00:00:00 2001 From: Joshua Batista <jbati...@microsoft.com> Date: Thu, 4 Apr 2024 14:25:36 -0700 Subject: [PATCH 03/10] 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 <jbati...@microsoft.com> Date: Wed, 10 Apr 2024 18:11:47 -0700 Subject: [PATCH 04/10] 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 <jbati...@microsoft.com> Date: Wed, 17 Apr 2024 19:45:14 -0700 Subject: [PATCH 05/10] 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@+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@+1 {{invalid register name prefix 'b' for register type 'RWBuffer' (expected 'u')}} -RWBuffer<int> a : register(b2, space1); - -// expected-error@+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 <jbati...@microsoft.com> Date: Thu, 18 Apr 2024 18:46:23 -0700 Subject: [PATCH 06/10] 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 <jbati...@microsoft.com> Date: Mon, 22 Apr 2024 10:30:09 -0700 Subject: [PATCH 07/10] 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 <jbati...@microsoft.com> Date: Tue, 23 Apr 2024 16:00:22 -0700 Subject: [PATCH 08/10] 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@+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@+1 {{invalid register name prefix 'u' for 'float' (expected 'b, c, or i)}} +float b : register(u0, space1); // expected-warning@+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@+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@+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@+1 {{invalid register name prefix 's' for register type 'cbuffer' (expected 'b')}} +// expected-error@+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@+1 {{invalid register name prefix 's' for register type 'tbuffer' (expected 'b')}} +// expected-error@+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 <jbati...@microsoft.com> Date: Tue, 23 Apr 2024 16:59:44 -0700 Subject: [PATCH 09/10] 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@+1 {{'register' attribute only applies to cbuffer/tbuffer and external global variables}} extern RWBuffer<float> U2 : register(u5); } -// expected-error@+1 {{invalid register name prefix 'u' for 'float' (expected 'b, c, or i)}} +// expected-error@+1 {{invalid register name prefix 'u' for 'float' (expected 't')}} float b : register(u0, space1); // expected-warning@+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 <jbati...@microsoft.com> Date: Wed, 24 Apr 2024 15:44:22 -0700 Subject: [PATCH 10/10] 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@+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@+1 {{invalid register name prefix 't' for register resource type 'RWBuffer' (expected 'u')}} RWBuffer<int> b : register(t2, space1); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits