[HLSL][RootSignature] Enable resource range analysis for remaining RootElements

[inbelic]

As implemented previously #140962.

We now have a validation pass to ensure that there is no overlap in the register ranges of the associated resources. However, in the previous pr, for the sake of brevity, we only "collected RangeInfo" for Root Descriptors. This means the analysis is not run on any other RootElement type.

This pr simply implements the collection of RangeInfo for the remaining types so that the analysis is run account for all RootElement types.

Additionally, we improve the diagnostics message to display unbounded ranges.

Part 3 of and Resolves #129942.

[llvmbot]

@llvm/pr-subscribers-hlsl

@llvm/pr-subscribers-clang

Author: Finn Plummer (inbelic)

Changes

As implemented previously #140962.

We now have a validation pass to ensure that there is no overlap in the register ranges of the associated resources. However, in the previous pr, for the sake of brevity, we only "collected RangeInfo" for Root Descriptors. This means the analysis is not run on any other RootElement type.

This pr simply implements the collection of RangeInfo for the remaining types so that the analysis is run account for all RootElement types.

Additionally, we improve the diagnostics message to display unbounded ranges.

Part 3 of and Resolves #140962.

---

Full diff: https://github.com/llvm/llvm-project/pull/145109.diff

4 Files Affected:

• (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+6-3)
• (modified) clang/lib/Sema/SemaHLSL.cpp (+49-1)
• (modified) clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl (+43)
• (modified) clang/test/SemaHLSL/RootSignature-resource-ranges.hlsl (+9)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index f2f2152b8bbbe..9392cbb39c021 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -13054,10 +13054,13 @@ def err_invalid_hlsl_resource_type: Error<
 def err_hlsl_spirv_only: Error<"%0 is only available for the SPIR-V target">;
 def err_hlsl_vk_literal_must_contain_constant: Error<"the argument to vk::Literal must be a vk::integral_constant">;
 
+def subst_hlsl_format_ranges: TextSubstitution<
+  "%select{t|u|b|s}0[%1;%select{%3]|unbounded)}2">;
+
 def err_hlsl_resource_range_overlap: Error<
-  "resource ranges %select{t|u|b|s}0[%1;%2] and %select{t|u|b|s}3[%4;%5] "
-  "overlap within space = %6 and visibility = "
-  "%select{All|Vertex|Hull|Domain|Geometry|Pixel|Amplification|Mesh}7">;
+  "resource ranges %sub{subst_hlsl_format_ranges}0,1,2,3 and %sub{subst_hlsl_format_ranges}4,5,6,7 "
+  "overlap within space = %8 and visibility = "
+  "%select{All|Vertex|Hull|Domain|Geometry|Pixel|Amplification|Mesh}9">;
 
 // Layout randomization diagnostics.
 def err_non_designated_init_used : Error<
diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index d003967a522a1..73f8bdf59abbb 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -1116,6 +1116,51 @@ bool SemaHLSL::handleRootSignatureDecl(HLSLRootSignatureDecl *D,
       Info.Space = Descriptor->Space;
       Info.Visibility = Descriptor->Visibility;
       Infos.push_back(Info);
+    } else if (const auto *Constants =
+                   std::get_if<llvm::hlsl::rootsig::RootConstants>(&Elem)) {
+      RangeInfo Info;
+      Info.LowerBound = Constants->Reg.Number;
+      Info.UpperBound = Info.LowerBound; // use inclusive ranges []
+
+      Info.Class = llvm::dxil::ResourceClass::CBuffer;
+      Info.Space = Constants->Space;
+      Info.Visibility = Constants->Visibility;
+      Infos.push_back(Info);
+    } else if (const auto *Sampler =
+                   std::get_if<llvm::hlsl::rootsig::StaticSampler>(&Elem)) {
+      RangeInfo Info;
+      Info.LowerBound = Sampler->Reg.Number;
+      Info.UpperBound = Info.LowerBound; // use inclusive ranges []
+
+      Info.Class = llvm::dxil::ResourceClass::Sampler;
+      Info.Space = Sampler->Space;
+      Info.Visibility = Sampler->Visibility;
+      Infos.push_back(Info);
+    } else if (const auto *Clause =
+                   std::get_if<llvm::hlsl::rootsig::DescriptorTableClause>(
+                       &Elem)) {
+      RangeInfo Info;
+      Info.LowerBound = Clause->Reg.Number;
+      assert(0 < Clause->NumDescriptors && "Verified as part of TODO(#129940)");
+      Info.UpperBound =
+        Clause->NumDescriptors == RangeInfo::Unbounded
+        ? RangeInfo::Unbounded
+        : Info.LowerBound + Clause->NumDescriptors - 1; // use inclusive ranges []
+
+      Info.Class = Clause->Type;
+      Info.Space = Clause->Space;
+      // Note: Clause does not hold the visibility this will need to
+      Infos.push_back(Info);
+    } else if (const auto *Table =
+                   std::get_if<llvm::hlsl::rootsig::DescriptorTable>(&Elem)) {
+      // Table holds the Visibility of all owned Clauses in Table, so iterate
+      // owned Clauses and update their corresponding RangeInfo
+      assert(Table->NumClauses <= Infos.size() && "RootElement");
+      // The last Table->NumClauses elements of Infos are the owned Clauses
+      // generated RangeInfo
+      auto TableInfos = MutableArrayRef<RangeInfo>(Infos).take_back(Table->NumClauses);
+      for (RangeInfo &Info : TableInfos)
+        Info.Visibility = Table->Visibility;
     }
   }
 
@@ -1159,8 +1204,11 @@ bool SemaHLSL::handleRootSignatureDecl(HLSLRootSignatureDecl *D,
             : Info->Visibility;
     this->Diag(Loc, diag::err_hlsl_resource_range_overlap)
         << llvm::to_underlying(Info->Class) << Info->LowerBound
+        << /*unbounded=*/(Info->UpperBound == RangeInfo::Unbounded)
         << Info->UpperBound << llvm::to_underlying(OInfo->Class)
-        << OInfo->LowerBound << OInfo->UpperBound << Info->Space << CommonVis;
+        << OInfo->LowerBound
+        << /*unbounded=*/(OInfo->UpperBound == RangeInfo::Unbounded)
+        << OInfo->UpperBound << Info->Space << CommonVis;
   };
 
   // 3: Iterate through collected RangeInfos
diff --git a/clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl b/clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl
index e5152e72d4806..4b3579d51818a 100644
--- a/clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl
+++ b/clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl
@@ -19,3 +19,46 @@ void bad_root_signature_3() {}
 // expected-error@+1 {{resource ranges u[0;0] and u[0;0] overlap within space = 0 and visibility = Pixel}}
 [RootSignature("UAV(u0, visibility = SHADER_VISIBILITY_PIXEL), UAV(u0, visibility = SHADER_VISIBILITY_ALL)")]
 void bad_root_signature_4() {}
+
+// expected-error@+1 {{resource ranges b[0;0] and b[0;0] overlap within space = 0 and visibility = All}}
+[RootSignature("RootConstants(num32BitConstants=4, b0), RootConstants(num32BitConstants=2, b0)")]
+void bad_root_signature_5() {}
+
+// expected-error@+1 {{resource ranges s[3;3] and s[3;3] overlap within space = 0 and visibility = All}}
+[RootSignature("StaticSampler(s3), StaticSampler(s3)")]
+void bad_root_signature_6() {}
+
+// expected-error@+1 {{resource ranges t[2;5] and t[0;3] overlap within space = 0 and visibility = All}}
+[RootSignature("DescriptorTable(SRV(t0, numDescriptors=4), SRV(t2, numDescriptors=4))")]
+void bad_root_signature_7() {}
+
+// expected-error@+1 {{resource ranges u[2;5] and u[0;unbounded) overlap within space = 0 and visibility = Hull}}
+[RootSignature("DescriptorTable(UAV(u0, numDescriptors=unbounded), visibility = SHADER_VISIBILITY_HULL), DescriptorTable(UAV(u2, numDescriptors=4))")]
+void bad_root_signature_8() {}
+
+// expected-error@+1 {{resource ranges b[0;2] and b[2;2] overlap within space = 0 and visibility = All}}
+[RootSignature("RootConstants(num32BitConstants=4, b2), DescriptorTable(CBV(b0, numDescriptors=3))")]
+void bad_root_signature_9() {}
+
+// expected-error@+1 {{resource ranges s[4;unbounded) and s[17;17] overlap within space = 0 and visibility = All}}
+[RootSignature("StaticSampler(s17), DescriptorTable(Sampler(s0, numDescriptors=3),Sampler(s4, numDescriptors=unbounded))")]
+void bad_root_signature_10() {}
+
+// expected-error@+1 {{resource ranges b[45;45] and b[4;unbounded) overlap within space = 0 and visibility = Geometry}}
+[RootSignature("DescriptorTable(CBV(b4, numDescriptors=unbounded)), CBV(b45, visibility = SHADER_VISIBILITY_GEOMETRY)")]
+void bad_root_signature_11() {}
+
+#define ReportFirstOverlap \
+ "DescriptorTable( " \
+ "  CBV(b4, numDescriptors = 4), " \
+ "  CBV(b1, numDescriptors = 2), " \
+ "  CBV(b0, numDescriptors = 8), " \
+ ")"
+
+// expected-error@+1 {{resource ranges b[0;7] and b[1;2] overlap within space = 0 and visibility = All}}
+[RootSignature(ReportFirstOverlap)]
+void bad_root_signature_12() {}
+
+// expected-error@+1 {{resource ranges s[2;2] and s[2;2] overlap within space = 0 and visibility = Vertex}}
+[RootSignature("StaticSampler(s2, visibility=SHADER_VISIBILITY_ALL), DescriptorTable(Sampler(s2), visibility=SHADER_VISIBILITY_VERTEX)")]
+void valid_root_signature_13() {}
diff --git a/clang/test/SemaHLSL/RootSignature-resource-ranges.hlsl b/clang/test/SemaHLSL/RootSignature-resource-ranges.hlsl
index 5778fb2ae4eb9..09a1110b0fbc1 100644
--- a/clang/test/SemaHLSL/RootSignature-resource-ranges.hlsl
+++ b/clang/test/SemaHLSL/RootSignature-resource-ranges.hlsl
@@ -13,3 +13,12 @@ void valid_root_signature_2() {}
 
 [RootSignature("CBV(b0), SRV(t0)")]
 void valid_root_signature_3() {}
+
+[RootSignature("RootConstants(num32BitConstants=4, b0, space=0), DescriptorTable(CBV(b0, space=1))")]
+void valid_root_signature_4() {}
+
+[RootSignature("StaticSampler(s2, visibility=SHADER_VISIBILITY_PIXEL), DescriptorTable(Sampler(s2), visibility=SHADER_VISIBILITY_VERTEX)")]
+void valid_root_signature_5() {}
+
+[RootSignature("DescriptorTable(SRV(t5), UAV(u5, numDescriptors=2))")]
+void valid_root_signature_6() {}

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.