[clang] Implement resource binding type prefix mismatch errors (PR #87578)

Joshua Batista via cfe-commits cfe-commits at lists.llvm.org
Mon Apr 22 10:30:29 PDT 2024


https://github.com/bob80905 updated https://github.com/llvm/llvm-project/pull/87578

>From 3960050439964fe3c0536696490b284a6c470cd1 Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Wed, 3 Apr 2024 13:15:59 -0700
Subject: [PATCH 1/7] implement binding type error for t/cbuffers and rwbuffers

---
 .../clang/Basic/DiagnosticSemaKinds.td        |  1 +
 clang/lib/Sema/HLSLExternalSemaSource.cpp     | 19 +++--
 clang/lib/Sema/SemaDeclAttr.cpp               | 73 ++++++++++++++++++-
 .../resource_binding_attr_error_mismatch.hlsl | 65 +++++++++++++++++
 4 files changed, 149 insertions(+), 9 deletions(-)
 create mode 100644 clang/test/SemaHLSL/resource_binding_attr_error_mismatch.hlsl

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 51af81bf1f6fc5..9a0946276f80fb 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12149,6 +12149,7 @@ def err_hlsl_missing_semantic_annotation : Error<
 def err_hlsl_init_priority_unsupported : Error<
   "initializer priorities are not supported in HLSL">;
 
+def err_hlsl_mismatching_register_type_and_name: Error<"invalid register name prefix '%0' for register type '%1' (expected '%2')">;
 def err_hlsl_unsupported_register_type : Error<"invalid resource class specifier '%0' used; expected 'b', 's', 't', or 'u'">;
 def err_hlsl_unsupported_register_number : Error<"register number should be an integer">;
 def err_hlsl_expected_space : Error<"invalid space specifier '%0' used; expected 'space' followed by an integer, like space1">;
diff --git a/clang/lib/Sema/HLSLExternalSemaSource.cpp b/clang/lib/Sema/HLSLExternalSemaSource.cpp
index 1a1febf7a35241..479689ec82dcee 100644
--- a/clang/lib/Sema/HLSLExternalSemaSource.cpp
+++ b/clang/lib/Sema/HLSLExternalSemaSource.cpp
@@ -119,8 +119,10 @@ struct BuiltinTypeDeclBuilder {
                                                 ResourceKind RK, bool IsROV) {
     if (Record->isCompleteDefinition())
       return *this;
-    Record->addAttr(HLSLResourceAttr::CreateImplicit(Record->getASTContext(),
-                                                     RC, RK, IsROV));
+    HLSLResourceAttr *attr = HLSLResourceAttr::CreateImplicit(
+        Record->getASTContext(), RC, RK, IsROV);
+
+    Record->addAttr(attr);
     return *this;
   }
 
@@ -482,15 +484,18 @@ static BuiltinTypeDeclBuilder setupBufferType(CXXRecordDecl *Decl, Sema &S,
                                               bool IsROV) {
   return BuiltinTypeDeclBuilder(Decl)
       .addHandleMember()
-      .addDefaultHandleConstructor(S, RC)
-      .annotateResourceClass(RC, RK, IsROV);
+      .addDefaultHandleConstructor(S, RC);
 }
 
 void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
   CXXRecordDecl *Decl;
-  Decl = BuiltinTypeDeclBuilder(*SemaPtr, HLSLNamespace, "RWBuffer")
-             .addSimpleTemplateParams({"element_type"})
-             .Record;
+  Decl =
+      BuiltinTypeDeclBuilder(*SemaPtr, HLSLNamespace, "RWBuffer")
+          .addSimpleTemplateParams({"element_type"})
+          .annotateResourceClass(ResourceClass::UAV, ResourceKind::TypedBuffer,
+                                 /*IsROV=*/false)
+          .Record;
+
   onCompletion(Decl, [this](CXXRecordDecl *Decl) {
     setupBufferType(Decl, *SemaPtr, ResourceClass::UAV,
                     ResourceKind::TypedBuffer, /*IsROV=*/false)
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index f25f3afd0f4af2..e720fe56c3ea17 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -7333,12 +7333,12 @@ static void handleHLSLResourceBindingAttr(Sema &S, Decl *D,
   } else {
     Slot = Str;
   }
-
+  QualType Ty = ((clang::ValueDecl *)D)->getType();
   // Validate.
   if (!Slot.empty()) {
     switch (Slot[0]) {
-    case 'u':
     case 'b':
+    case 'u':
     case 's':
     case 't':
       break;
@@ -7367,6 +7367,75 @@ static void handleHLSLResourceBindingAttr(Sema &S, Decl *D,
     return;
   }
 
+  VarDecl *VD = dyn_cast<VarDecl>(D);
+  HLSLBufferDecl *BD = dyn_cast<HLSLBufferDecl>(D);
+
+  if (VD || BD) {
+    llvm::hlsl::ResourceClass RC;
+    std::string varTy = "";
+    if (VD) {
+
+      const Type *Ty = VD->getType()->getPointeeOrArrayElementType();
+      if (!Ty)
+        return;
+      QualType t = ((ElaboratedType *)Ty)->getNamedType();
+      const CXXRecordDecl *RD = Ty->getAsCXXRecordDecl();
+      if (!RD)
+        return;
+
+      if (auto TDecl = dyn_cast<ClassTemplateSpecializationDecl>(RD))
+        RD = TDecl->getSpecializedTemplate()->getTemplatedDecl();
+      RD = RD->getCanonicalDecl();
+      const auto *Attr = RD->getAttr<HLSLResourceAttr>();
+      if (!Attr)
+        return;
+
+      RC = Attr->getResourceClass();
+      varTy = RD->getNameAsString();
+    } else {
+      if (BD->isCBuffer()) {
+        RC = llvm::hlsl::ResourceClass::CBuffer;
+        varTy = "cbuffer";
+      } else {
+        RC = llvm::hlsl::ResourceClass::CBuffer;
+        varTy = "tbuffer";
+      }
+    }
+    switch (RC) {
+    case llvm::hlsl::ResourceClass::SRV: {
+      if (Slot.substr(0, 1) != "t")
+        S.Diag(ArgLoc, diag::err_hlsl_mismatching_register_type_and_name)
+            << Slot.substr(0, 1) << varTy << "t";
+      break;
+    }
+    case llvm::hlsl::ResourceClass::UAV: {
+      if (Slot.substr(0, 1) != "u")
+        S.Diag(ArgLoc, diag::err_hlsl_mismatching_register_type_and_name)
+            << Slot.substr(0, 1) << varTy << "u";
+      break;
+    }
+    case llvm::hlsl::ResourceClass::CBuffer: {
+      if (Slot.substr(0, 1) != "b")
+        S.Diag(ArgLoc, diag::err_hlsl_mismatching_register_type_and_name)
+            << Slot.substr(0, 1) << varTy << "b";
+      break;
+    }
+    case llvm::hlsl::ResourceClass::Sampler: {
+      if (Slot.substr(0, 1) != "s")
+        S.Diag(ArgLoc, diag::err_hlsl_mismatching_register_type_and_name)
+            << Slot.substr(0, 1) << varTy << "s";
+      break;
+    }
+    case llvm::hlsl::ResourceClass::Invalid: {
+      llvm_unreachable("Resource class should be valid.");
+      break;
+    }
+
+    default:
+      break;
+    }
+  }
+
   // FIXME: check reg type match decl. Issue
   // https://github.com/llvm/llvm-project/issues/57886.
   HLSLResourceBindingAttr *NewAttr =
diff --git a/clang/test/SemaHLSL/resource_binding_attr_error_mismatch.hlsl b/clang/test/SemaHLSL/resource_binding_attr_error_mismatch.hlsl
new file mode 100644
index 00000000000000..76dcbd201cbd19
--- /dev/null
+++ b/clang/test/SemaHLSL/resource_binding_attr_error_mismatch.hlsl
@@ -0,0 +1,65 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -o - -fsyntax-only %s -verify
+
+
+// expected-error at +1 {{invalid register name prefix 'b' for register type 'RWBuffer' (expected 'u')}}
+RWBuffer<int> a : register(b2, space1);
+
+// expected-error at +1 {{invalid register name prefix 't' for register type 'RWBuffer' (expected 'u')}}
+RWBuffer<int> b : register(t2, space1);
+
+// NOT YET IMPLEMENTED : {{invalid register name prefix 'u' for register type 'Texture1D' (expected 't')}}
+// NOT YET IMPLEMENTED Texture1D<float> tex : register(u3);
+
+// NOT YET IMPLEMENTED : {{invalid register name prefix 's' for register type 'Texture2D' (expected 't')}}
+// NOT YET IMPLEMENTED Texture2D<float> Texture : register(s0);
+
+// NOT YET IMPLEMENTED : {{invalid register name prefix 'u' for register type 'Texture2DMS' (expected 't')}}
+// NOT YET IMPLEMENTED Texture2DMS<float4, 4> T2DMS_t2 : register(u2)
+
+// NOT YET IMPLEMENTED : {{invalid register name prefix 't' for register type 'RWTexture3D' (expected 'u')}}
+// NOT YET IMPLEMENTED RWTexture3D<float4> RWT3D_u1 : register(t1)
+
+// NOT YET IMPLEMENTED : {{invalid register name prefix 'b' for register type 'Texture2DMS' (expected 't' or 's')}}
+// NOT YET IMPLEMENTED TextureCube TCube_b2 : register(B2);
+
+// NOT YET IMPLEMENTED : {{invalid register name prefix 'b' for register type 'Texture2DMS' (expected 't')}}
+// NOT YET IMPLEMENTED TextureCubeArray TCubeArray_t2 : register(b2);
+
+// NOT YET IMPLEMENTED : {{invalid register name prefix 'b' for register type 'Texture2DMS' (expected 't' or 's')}}
+// NOT YET IMPLEMENTED Texture1DArray T1DArray_t2 : register(b2);
+
+// NOT YET IMPLEMENTED : {{invalid register name prefix 'u' for register type 'Texture2DMS' (expected 't' or 's')}}
+// NOT YET IMPLEMENTED Texture2DArray T2DArray_b2 : register(B2);
+
+// NOT YET IMPLEMENTED : {{invalid register name prefix 'u' for register type 'Texture2DMS' (expected 'b' or 'c' or 'i')}}
+// NOT YET IMPLEMENTED Texture2DMSArray<float4> msTextureArray : register(t2, space2);
+
+// NOT YET IMPLEMENTED : {{invalid register name prefix 'u' for register type 'TCubeArray_f2' (expected 't' or 's')}}
+// NOT YET IMPLEMENTED TextureCubeArray TCubeArray_f2 : register(u2);
+
+// NOT YET IMPLEMENTED : {{invalid register name prefix 'u' for register type 'TypedBuffer' (expected 't')}}
+// NOT YET IMPLEMENTED TypedBuffer tbuf : register(u2);
+
+// NOT YET IMPLEMENTED : {{invalid register name prefix 'u' for register type 'RawBuffer' (expected 't')}}
+// NOT YET IMPLEMENTED RawBuffer rbuf : register(u2);
+
+// NOT YET IMPLEMENTED : {{invalid register name prefix 't' for register type 'StructuredBuffer' (expected 'u')}}
+// NOT YET IMPLEMENTED StructuredBuffer ROVStructuredBuff_t2  : register(T2);
+
+// expected-error at +1 {{invalid register name prefix 's' for register type 'cbuffer' (expected 'b')}}
+cbuffer f : register(s2, space1) {}
+
+// NOT YET IMPLEMENTED : {{invalid register name prefix 't' for register type 'Sampler' (expected 's')}}
+// Can this type just be Sampler instead of SamplerState?
+// NOT YET IMPLEMENTED SamplerState MySampler : register(t3, space1);
+
+// expected-error at +1 {{invalid register name prefix 's' for register type 'tbuffer' (expected 'b')}}
+tbuffer f : register(s2, space1) {}
+
+// NOT YET IMPLEMENTED : RTAccelerationStructure doesn't have any example tests in DXC
+
+// NOT YET IMPLEMENTED : {{invalid register name prefix 'u' for register type 'FeedbackTexture2D' (expected 'b' or 'c' or 'i')}}
+// NOT YET IMPLEMENTED FeedbackTexture2D<float> FBTex2D[3][] : register(u0, space26);
+
+// NOT YET IMPLEMENTED : {{invalid register name prefix 'u' for register type 'FeedbackTexture2DArray' (expected 'b' or 'c' or 'i')}}
+// NOT YET IMPLEMENTED FeedbackTexture2DArray<float> FBTex2DArr[3][2][] : register(u0, space27);

>From c307d0e2950976b1862db2a6d3d0005913da4f55 Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Thu, 4 Apr 2024 14:04:58 -0700
Subject: [PATCH 2/7] 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 3/7] 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 4/7] 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 5/7] 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 6/7] 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 7/7] 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;



More information about the cfe-commits mailing list