[clang] [HLSL] Add testing for space parameter on global constants (PR #106782)

Joshua Batista via cfe-commits cfe-commits at lists.llvm.org
Mon Sep 16 10:53:09 PDT 2024


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

>From 99408f31a8946df7ef9efa223d0dba2ab876fcd0 Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Fri, 30 Aug 2024 12:08:37 -0700
Subject: [PATCH 1/6] add diag and testing for space and global constants

---
 .../clang/Basic/DiagnosticSemaKinds.td        |  1 +
 clang/lib/Sema/SemaHLSL.cpp                   | 16 +++++++++---
 .../SemaHLSL/resource_binding_attr_error.hlsl | 17 +++----------
 .../resource_binding_attr_error_basic.hlsl    | 21 +++++++++-------
 .../resource_binding_attr_error_space.hlsl    | 25 +++++++++++++++++++
 5 files changed, 53 insertions(+), 27 deletions(-)
 create mode 100644 clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index edf22b909c4d57..e099cb954cac75 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12357,6 +12357,7 @@ def warn_hlsl_deprecated_register_type_b: Warning<"binding type 'b' only applies
 def warn_hlsl_deprecated_register_type_i: Warning<"binding type 'i' ignored. The 'integer constant' binding type is no longer supported">, InGroup<LegacyConstantRegisterBinding>, DefaultError;
 def err_hlsl_unsupported_register_number : Error<"register number should be an integer">;
 def err_hlsl_expected_space : Error<"invalid space specifier '%0' used; expected 'space' followed by an integer, like space1">;
+def err_hlsl_space_on_global_constant : Error<"register space cannot be specified on global constants">;
 def warn_hlsl_packoffset_mix : Warning<"cannot mix packoffset elements with nonpackoffset elements in a cbuffer">,
     InGroup<HLSLMixPackOffset>;
 def err_hlsl_packoffset_overlap : Error<"packoffset overlap between %0, %1">;
diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index 714e8f5cfa9926..61bed66445dc78 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -722,7 +722,8 @@ static void ValidateMultipleRegisterAnnotations(Sema &S, Decl *TheDecl,
 }
 
 static void DiagnoseHLSLRegisterAttribute(Sema &S, SourceLocation &ArgLoc,
-                                          Decl *TheDecl, RegisterType regType) {
+                                          Decl *TheDecl, RegisterType regType,
+                                          StringRef SpaceNum) {
 
   // Samplers, UAVs, and SRVs are VarDecl types
   VarDecl *TheVarDecl = dyn_cast<VarDecl>(TheDecl);
@@ -795,6 +796,9 @@ static void DiagnoseHLSLRegisterAttribute(Sema &S, SourceLocation &ArgLoc,
         S.Diag(ArgLoc, diag::warn_hlsl_deprecated_register_type_b);
       else if (regType != RegisterType::C)
         S.Diag(ArgLoc, diag::err_hlsl_binding_type_mismatch) << regTypeNum;
+      // non-zero SpaceNum cannot be specified for global constants
+      if (SpaceNum != "0")
+        S.Diag(ArgLoc, diag::err_hlsl_space_on_global_constant);
       return;
     }
 
@@ -817,8 +821,12 @@ static void DiagnoseHLSLRegisterAttribute(Sema &S, SourceLocation &ArgLoc,
       S.Diag(TheDecl->getLocation(),
              diag::warn_hlsl_user_defined_type_missing_member)
           << regTypeNum;
-
-    return;
+    // non-zero SpaceNum cannot be specified for global constants
+    if (!isDeclaredWithinCOrTBuffer(TheDecl)) {
+      if (SpaceNum != "0")
+        S.Diag(ArgLoc, diag::err_hlsl_space_on_global_constant);
+      return;
+    }
   }
 }
 
@@ -891,7 +899,7 @@ void SemaHLSL::handleResourceBindingAttr(Decl *TheDecl, const ParsedAttr &AL) {
     return;
   }
 
-  DiagnoseHLSLRegisterAttribute(SemaRef, ArgLoc, TheDecl, regType);
+  DiagnoseHLSLRegisterAttribute(SemaRef, ArgLoc, TheDecl, regType, SpaceNum);
 
   HLSLResourceBindingAttr *NewAttr =
       HLSLResourceBindingAttr::Create(getASTContext(), Slot, Space, AL);
diff --git a/clang/test/SemaHLSL/resource_binding_attr_error.hlsl b/clang/test/SemaHLSL/resource_binding_attr_error.hlsl
index 6a0b5956545dd8..823b75f14d1957 100644
--- a/clang/test/SemaHLSL/resource_binding_attr_error.hlsl
+++ b/clang/test/SemaHLSL/resource_binding_attr_error.hlsl
@@ -13,16 +13,9 @@ float a : register(c0);
 cbuffer b : register(i0) {
 
 }
-// expected-error at +1 {{invalid space specifier 's2' used; expected 'space' followed by an integer, like space1}}
-cbuffer c : register(b0, s2) {
 
-}
 // expected-error at +1 {{register number should be an integer}}
-cbuffer d : register(bf, s2) {
-
-}
-// expected-error at +1 {{invalid space specifier 'spaces' used; expected 'space' followed by an integer, like space1}}
-cbuffer e : register(b2, spaces) {
+cbuffer c : register(bf, s2) {
 
 }
 
@@ -35,9 +28,8 @@ cbuffer B : register(space1) {}
 // expected-error at +1 {{wrong argument format for hlsl attribute, use b2 instead}}
 cbuffer C : register(b 2) {}
 
-// expected-error at +2 {{wrong argument format for hlsl attribute, use b2 instead}}
-// expected-error at +1 {{wrong argument format for hlsl attribute, use space3 instead}}
-cbuffer D : register(b 2, space 3) {}
+// expected-error at +1 {{wrong argument format for hlsl attribute, use b2 instead}}
+cbuffer D : register(b 2, space3) {}
 
 // expected-error at +1 {{'register' attribute only applies to cbuffer/tbuffer and external global variables}}
 static MyTemplatedSRV<float> U : register(u5);
@@ -61,9 +53,6 @@ void foo2() {
   extern MyTemplatedSRV<float> U2 : register(u5);
 }
 
-// expected-error at +1 {{binding type 'u' only applies to UAV resources}}
-float b : register(u0, space1);
-
 // expected-error at +1 {{'register' attribute only applies to cbuffer/tbuffer and external global variables}}
 void bar(MyTemplatedSRV<float> U : register(u3)) {
 
diff --git a/clang/test/SemaHLSL/resource_binding_attr_error_basic.hlsl b/clang/test/SemaHLSL/resource_binding_attr_error_basic.hlsl
index 0a547ed66af0a2..760c057630a7fa 100644
--- a/clang/test/SemaHLSL/resource_binding_attr_error_basic.hlsl
+++ b/clang/test/SemaHLSL/resource_binding_attr_error_basic.hlsl
@@ -3,37 +3,40 @@
 // expected-error at +1{{binding type 't' only applies to SRV resources}}
 float f1 : register(t0);
 
-
-float f2 : register(c0);
+// expected-error at +1 {{binding type 'u' only applies to UAV resources}}
+float f2 : register(u0);
 
 // expected-error at +1{{binding type 'b' only applies to constant buffers. The 'bool constant' binding type is no longer supported}}
 float f3 : register(b9);
 
+// expected-error at +1 {{binding type 's' only applies to sampler state}}
+float f4 : register(s0);
+
 // expected-error at +1{{binding type 'i' ignored. The 'integer constant' binding type is no longer supported}}
-float f4 : register(i9);
+float f5 : register(i9);
 
 // expected-error at +1{{binding type 'x' is invalid}}
-float f5 : register(x9);
+float f6 : register(x9);
 
 cbuffer g_cbuffer1 {
 // expected-error at +1{{binding type 'c' ignored in buffer declaration. Did you mean 'packoffset'?}}
-    float f6 : register(c2);
+    float f7 : register(c2);
 };
 
 tbuffer g_tbuffer1 {
 // expected-error at +1{{binding type 'c' ignored in buffer declaration. Did you mean 'packoffset'?}}
-    float f7 : register(c2);
+    float f8 : register(c2);
 };
 
 cbuffer g_cbuffer2 {
 // expected-error at +1{{binding type 'b' only applies to constant buffer resources}}
-    float f8 : register(b2);
+    float f9 : register(b2);
 };
 
 tbuffer g_tbuffer2 {
 // expected-error at +1{{binding type 'i' ignored. The 'integer constant' binding type is no longer supported}}
-    float f9 : register(i2);
+    float f10 : register(i2);
 };
 
 // expected-error at +1{{binding type 'c' only applies to numeric variables in the global scope}}
-RWBuffer<float> f10 : register(c3);
+RWBuffer<float> f11 : register(c3);
diff --git a/clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl b/clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl
new file mode 100644
index 00000000000000..2f1b19e1bfc696
--- /dev/null
+++ b/clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -o - -fsyntax-only %s -verify
+
+
+// expected-error at +1 {{invalid space specifier 's2' used; expected 'space' followed by an integer, like space1}}
+cbuffer a : register(b0, s2) {
+
+}
+
+// expected-error at +1 {{invalid space specifier 'spaces' used; expected 'space' followed by an integer, like space1}}
+cbuffer b : register(b2, spaces) {
+
+}
+
+// expected-error at +1 {{wrong argument format for hlsl attribute, use space3 instead}}
+cbuffer c : register(b2, space 3) {}
+
+// expected-error at +1 {{register space cannot be specified on global constants}}
+int d : register(c2, space3);
+
+struct S {
+    RWBuffer<int> a;
+};
+
+// expected-error at +1 {{register space cannot be specified on global constants}}
+S thing : register(u2, space2);
\ No newline at end of file

>From 9fb3c96ef1027422953e9d2a434723ff3481c5df Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Fri, 30 Aug 2024 12:23:20 -0700
Subject: [PATCH 2/6] add newline

---
 clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl b/clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl
index 2f1b19e1bfc696..55f1fa50fac763 100644
--- a/clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl
+++ b/clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl
@@ -22,4 +22,4 @@ struct S {
 };
 
 // expected-error at +1 {{register space cannot be specified on global constants}}
-S thing : register(u2, space2);
\ No newline at end of file
+S thing : register(u2, space2);

>From 59218ac2b3a5c1102060b32ba22eae36094db303 Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Fri, 30 Aug 2024 13:38:55 -0700
Subject: [PATCH 3/6] detect space param properly, add more tests

---
 clang/lib/Sema/SemaHLSL.cpp                          | 12 ++++++------
 .../SemaHLSL/resource_binding_attr_error_space.hlsl  | 12 ++++++++++++
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index 61bed66445dc78..6fb5bbd3db961a 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -723,7 +723,7 @@ static void ValidateMultipleRegisterAnnotations(Sema &S, Decl *TheDecl,
 
 static void DiagnoseHLSLRegisterAttribute(Sema &S, SourceLocation &ArgLoc,
                                           Decl *TheDecl, RegisterType regType,
-                                          StringRef SpaceNum) {
+                                          const ParsedAttr &AL) {
 
   // Samplers, UAVs, and SRVs are VarDecl types
   VarDecl *TheVarDecl = dyn_cast<VarDecl>(TheDecl);
@@ -796,8 +796,8 @@ static void DiagnoseHLSLRegisterAttribute(Sema &S, SourceLocation &ArgLoc,
         S.Diag(ArgLoc, diag::warn_hlsl_deprecated_register_type_b);
       else if (regType != RegisterType::C)
         S.Diag(ArgLoc, diag::err_hlsl_binding_type_mismatch) << regTypeNum;
-      // non-zero SpaceNum cannot be specified for global constants
-      if (SpaceNum != "0")
+      // Space argument cannot be specified for global constants
+      if (AL.getNumArgs() == 2)
         S.Diag(ArgLoc, diag::err_hlsl_space_on_global_constant);
       return;
     }
@@ -821,9 +821,9 @@ static void DiagnoseHLSLRegisterAttribute(Sema &S, SourceLocation &ArgLoc,
       S.Diag(TheDecl->getLocation(),
              diag::warn_hlsl_user_defined_type_missing_member)
           << regTypeNum;
-    // non-zero SpaceNum cannot be specified for global constants
+    // Space argument cannot be specified for global constants
     if (!isDeclaredWithinCOrTBuffer(TheDecl)) {
-      if (SpaceNum != "0")
+      if (AL.getNumArgs() == 2)
         S.Diag(ArgLoc, diag::err_hlsl_space_on_global_constant);
       return;
     }
@@ -899,7 +899,7 @@ void SemaHLSL::handleResourceBindingAttr(Decl *TheDecl, const ParsedAttr &AL) {
     return;
   }
 
-  DiagnoseHLSLRegisterAttribute(SemaRef, ArgLoc, TheDecl, regType, SpaceNum);
+  DiagnoseHLSLRegisterAttribute(SemaRef, ArgLoc, TheDecl, regType, AL);
 
   HLSLResourceBindingAttr *NewAttr =
       HLSLResourceBindingAttr::Create(getASTContext(), Slot, Space, AL);
diff --git a/clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl b/clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl
index 55f1fa50fac763..892839384302ec 100644
--- a/clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl
+++ b/clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl
@@ -17,6 +17,18 @@ cbuffer c : register(b2, space 3) {}
 // expected-error at +1 {{register space cannot be specified on global constants}}
 int d : register(c2, space3);
 
+// expected-error at +1 {{register space cannot be specified on global constants}}
+int e : register(c2, space0);
+
+// expected-error at +1 {{register space cannot be specified on global constants}}
+int f : register(c2, space00);
+
+// valid
+RWBuffer<int> g : register(u2, space0);
+
+// valid
+RWBuffer<int> h : register(u2, space0);
+
 struct S {
     RWBuffer<int> a;
 };

>From 1c1943db597e762162fb01773fc1045a15c8e54a Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Thu, 12 Sep 2024 17:02:03 -0700
Subject: [PATCH 4/6] address damyan, simplify logic

---
 clang/lib/Sema/SemaHLSL.cpp                   | 51 ++++++++++---------
 .../resource_binding_attr_error_space.hlsl    | 19 +++++++
 2 files changed, 45 insertions(+), 25 deletions(-)

diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index 6fb5bbd3db961a..eb516898f6586c 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -722,8 +722,8 @@ static void ValidateMultipleRegisterAnnotations(Sema &S, Decl *TheDecl,
 }
 
 static void DiagnoseHLSLRegisterAttribute(Sema &S, SourceLocation &ArgLoc,
-                                          Decl *TheDecl, RegisterType regType,
-                                          const ParsedAttr &AL) {
+                                          Decl *TheDecl, RegisterType RegType,
+                                          const bool SpecifiedSpace) {
 
   // Samplers, UAVs, and SRVs are VarDecl types
   VarDecl *TheVarDecl = dyn_cast<VarDecl>(TheDecl);
@@ -741,16 +741,23 @@ static void DiagnoseHLSLRegisterAttribute(Sema &S, SourceLocation &ArgLoc,
              1 &&
          "only one resource analysis result should be expected");
 
-  int regTypeNum = static_cast<int>(regType);
+  int RegTypeNum = static_cast<int>(RegType);
 
   // first, if "other" is set, emit an error
   if (Flags.Other) {
-    S.Diag(ArgLoc, diag::err_hlsl_binding_type_mismatch) << regTypeNum;
+    S.Diag(ArgLoc, diag::err_hlsl_binding_type_mismatch) << RegTypeNum;
+    return;
+  }
+
+  // Space argument cannot be specified for global constants
+  if ((Flags.DefaultGlobals || Flags.UDT) &&
+      !isDeclaredWithinCOrTBuffer(TheDecl) && SpecifiedSpace) {
+    S.Diag(ArgLoc, diag::err_hlsl_space_on_global_constant);
     return;
   }
 
   // next, if multiple register annotations exist, check that none conflict.
-  ValidateMultipleRegisterAnnotations(S, TheDecl, regType);
+  ValidateMultipleRegisterAnnotations(S, TheDecl, RegType);
 
   // next, if resource is set, make sure the register type in the register
   // annotation is compatible with the variable's resource type.
@@ -782,9 +789,9 @@ static void DiagnoseHLSLRegisterAttribute(Sema &S, SourceLocation &ArgLoc,
 
     RegisterType ExpectedRegisterType =
         ExpectedRegisterTypesForResourceClass[(int)DeclResourceClass];
-    if (regType != ExpectedRegisterType) {
+    if (RegType != ExpectedRegisterType) {
       S.Diag(TheDecl->getLocation(), diag::err_hlsl_binding_type_mismatch)
-          << regTypeNum;
+          << RegTypeNum;
     }
     return;
   }
@@ -792,20 +799,17 @@ static void DiagnoseHLSLRegisterAttribute(Sema &S, SourceLocation &ArgLoc,
   // next, handle diagnostics for when the "basic" flag is set
   if (Flags.Basic) {
     if (Flags.DefaultGlobals) {
-      if (regType == RegisterType::CBuffer)
+      if (RegType == RegisterType::CBuffer)
         S.Diag(ArgLoc, diag::warn_hlsl_deprecated_register_type_b);
-      else if (regType != RegisterType::C)
-        S.Diag(ArgLoc, diag::err_hlsl_binding_type_mismatch) << regTypeNum;
-      // Space argument cannot be specified for global constants
-      if (AL.getNumArgs() == 2)
-        S.Diag(ArgLoc, diag::err_hlsl_space_on_global_constant);
+      else if (RegType != RegisterType::C)
+        S.Diag(ArgLoc, diag::err_hlsl_binding_type_mismatch) << RegTypeNum;
       return;
     }
 
-    if (regType == RegisterType::C)
+    if (RegType == RegisterType::C)
       S.Diag(ArgLoc, diag::warn_hlsl_register_type_c_packoffset);
     else
-      S.Diag(ArgLoc, diag::err_hlsl_binding_type_mismatch) << regTypeNum;
+      S.Diag(ArgLoc, diag::err_hlsl_binding_type_mismatch) << RegTypeNum;
 
     return;
   }
@@ -814,19 +818,13 @@ static void DiagnoseHLSLRegisterAttribute(Sema &S, SourceLocation &ArgLoc,
   if (Flags.UDT) {
     const bool ExpectedRegisterTypesForUDT[] = {
         Flags.SRV, Flags.UAV, Flags.CBV, Flags.Sampler, Flags.ContainsNumeric};
-    assert((size_t)regTypeNum < std::size(ExpectedRegisterTypesForUDT) &&
+    assert((size_t)RegTypeNum < std::size(ExpectedRegisterTypesForUDT) &&
            "regType has unexpected value");
 
-    if (!ExpectedRegisterTypesForUDT[regTypeNum])
+    if (!ExpectedRegisterTypesForUDT[RegTypeNum])
       S.Diag(TheDecl->getLocation(),
              diag::warn_hlsl_user_defined_type_missing_member)
-          << regTypeNum;
-    // Space argument cannot be specified for global constants
-    if (!isDeclaredWithinCOrTBuffer(TheDecl)) {
-      if (AL.getNumArgs() == 2)
-        S.Diag(ArgLoc, diag::err_hlsl_space_on_global_constant);
-      return;
-    }
+          << RegTypeNum;
   }
 }
 
@@ -851,7 +849,9 @@ void SemaHLSL::handleResourceBindingAttr(Decl *TheDecl, const ParsedAttr &AL) {
   SourceLocation ArgLoc = Loc->Loc;
 
   SourceLocation SpaceArgLoc;
+  bool SpecifiedSpace = false;
   if (AL.getNumArgs() == 2) {
+    SpecifiedSpace = true;
     Slot = Str;
     if (!AL.isArgIdent(1)) {
       Diag(AL.getLoc(), diag::err_attribute_argument_type)
@@ -899,7 +899,8 @@ void SemaHLSL::handleResourceBindingAttr(Decl *TheDecl, const ParsedAttr &AL) {
     return;
   }
 
-  DiagnoseHLSLRegisterAttribute(SemaRef, ArgLoc, TheDecl, regType, AL);
+  DiagnoseHLSLRegisterAttribute(SemaRef, ArgLoc, TheDecl, regType,
+                                SpecifiedSpace);
 
   HLSLResourceBindingAttr *NewAttr =
       HLSLResourceBindingAttr::Create(getASTContext(), Slot, Space, AL);
diff --git a/clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl b/clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl
index 892839384302ec..bd97b61e5d6e00 100644
--- a/clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl
+++ b/clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl
@@ -1,5 +1,24 @@
 // RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -o - -fsyntax-only %s -verify
 
+// valid
+cbuffer cbuf {
+    RWBuffer<int> r : register(u0, space0);
+}
+
+cbuffer cbuf2 {
+    struct x {
+        // expected-error at +1 {{'register' attribute only applies to cbuffer/tbuffer and external global variables}}
+        RWBuffer<int> E : register(u2, space3);
+    };
+}
+
+struct MyStruct {
+    RWBuffer<int> E;
+};
+
+cbuffer cbuf3 {
+  MyStruct E : register(u2, space3);
+}
 
 // expected-error at +1 {{invalid space specifier 's2' used; expected 'space' followed by an integer, like space1}}
 cbuffer a : register(b0, s2) {

>From 83cc59d2392163a7dbb4db91050a40afad604d72 Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Fri, 13 Sep 2024 09:38:04 -0700
Subject: [PATCH 5/6] add more tests

---
 .../SemaHLSL/resource_binding_attr_error_space.hlsl  | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl b/clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl
index bd97b61e5d6e00..f40c12e52a577a 100644
--- a/clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl
+++ b/clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl
@@ -7,6 +7,8 @@ cbuffer cbuf {
 
 cbuffer cbuf2 {
     struct x {
+        // this test validates that no diagnostic is emitted on the space parameter, because
+        // this register annotation is not in the global scope.
         // expected-error at +1 {{'register' attribute only applies to cbuffer/tbuffer and external global variables}}
         RWBuffer<int> E : register(u2, space3);
     };
@@ -20,6 +22,16 @@ cbuffer cbuf3 {
   MyStruct E : register(u2, space3);
 }
 
+// expected-error at +1 {{register space cannot be specified on global constants}}
+MyStruct F : register(u3, space4);
+
+cbuffer cbuf4 {
+  // this test validates that no diagnostic is emitted on the space parameter, because
+  // this register annotation is not in the global scope.
+  // expected-error at +1 {{binding type 'u' only applies to UAV resources}}
+  float a : register(u2, space3); 
+}
+
 // expected-error at +1 {{invalid space specifier 's2' used; expected 'space' followed by an integer, like space1}}
 cbuffer a : register(b0, s2) {
 

>From 35aa6d4ddb56ddb4f158e0f5e263875a86711884 Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Mon, 16 Sep 2024 10:52:48 -0700
Subject: [PATCH 6/6] no error on UDTs

---
 clang/lib/Sema/SemaHLSL.cpp                           | 11 ++++-------
 .../SemaHLSL/resource_binding_attr_error_space.hlsl   |  6 ++----
 2 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index 433089f956b519..c0c628f24fd1f2 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -931,13 +931,6 @@ static void DiagnoseHLSLRegisterAttribute(Sema &S, SourceLocation &ArgLoc,
     return;
   }
 
-  // Space argument cannot be specified for global constants
-  if ((Flags.DefaultGlobals || Flags.UDT) &&
-      !isDeclaredWithinCOrTBuffer(TheDecl) && SpecifiedSpace) {
-    S.Diag(ArgLoc, diag::err_hlsl_space_on_global_constant);
-    return;
-  }
-
   // next, if multiple register annotations exist, check that none conflict.
   ValidateMultipleRegisterAnnotations(S, TheDecl, RegType);
 
@@ -949,11 +942,15 @@ static void DiagnoseHLSLRegisterAttribute(Sema &S, SourceLocation &ArgLoc,
       S.Diag(TheDecl->getLocation(), diag::err_hlsl_binding_type_mismatch)
           << RegTypeNum;
     }
+
     return;
   }
 
   // next, handle diagnostics for when the "basic" flag is set
   if (Flags.Basic) {
+    if (SpecifiedSpace && !isDeclaredWithinCOrTBuffer(TheDecl))
+      S.Diag(ArgLoc, diag::err_hlsl_space_on_global_constant);
+
     if (Flags.DefaultGlobals) {
       if (RegType == RegisterType::CBuffer)
         S.Diag(ArgLoc, diag::warn_hlsl_deprecated_register_type_b);
diff --git a/clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl b/clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl
index f40c12e52a577a..fe16e0269db5ae 100644
--- a/clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl
+++ b/clang/test/SemaHLSL/resource_binding_attr_error_space.hlsl
@@ -19,10 +19,11 @@ struct MyStruct {
 };
 
 cbuffer cbuf3 {
+  // valid
   MyStruct E : register(u2, space3);
 }
 
-// expected-error at +1 {{register space cannot be specified on global constants}}
+// valid
 MyStruct F : register(u3, space4);
 
 cbuffer cbuf4 {
@@ -63,6 +64,3 @@ RWBuffer<int> h : register(u2, space0);
 struct S {
     RWBuffer<int> a;
 };
-
-// expected-error at +1 {{register space cannot be specified on global constants}}
-S thing : register(u2, space2);



More information about the cfe-commits mailing list