[llvm-branch-commits] [llvm] [DirectX] Adding missing descriptor table validations (PR #153276)

[Finn Plummer via llvm-branch-commits]

================
@@ -513,6 +515,48 @@ Error MetadataParser::parseRootSignatureElement(mcdxbc::RootSignatureDesc &RSD,
   llvm_unreachable("Unhandled RootSignatureElementKind enum.");
 }
 
+Error validateDescriptorTableSamplerMixin(mcdxbc::DescriptorTable Table,
+                                          uint32_t Location) {
+  bool HasSampler = false;
+  bool HasOtherRangeType = false;
+  dxbc::DescriptorRangeType OtherRangeType;
+
+  for (const dxbc::RTS0::v2::DescriptorRange &Range : Table.Ranges) {
+    dxbc::DescriptorRangeType RangeType =
+        static_cast<dxbc::DescriptorRangeType>(Range.RangeType);
+
+    if (RangeType == dxbc::DescriptorRangeType::Sampler) {
+      HasSampler = true;
+    } else {
+      HasOtherRangeType = true;
+      OtherRangeType = RangeType;
+    }
+  }
+
+  // Samplers cannot be mixed with other resources in a descriptor table.
+  if (HasSampler && HasOtherRangeType)
+    return make_error<TableSamplerMixinError>(OtherRangeType, Location);
+  return Error::success();
+}
+
+Error validateDescriptorTableRegisterOverflow(mcdxbc::DescriptorTable Table,
+                                              uint32_t Location) {
+  uint64_t AppendingRegister = 0;
+
+  for (const dxbc::RTS0::v2::DescriptorRange &Range : Table.Ranges) {
+    dxbc::DescriptorRangeType RangeType =
+        static_cast<dxbc::DescriptorRangeType>(Range.RangeType);
+    if (verifyOffsetOverflowing(AppendingRegister,
+                                Range.OffsetInDescriptorsFromTableStart,
+                                Range.BaseShaderRegister, Range.RegisterSpace,
+                                Range.NumDescriptors))
+      return make_error<TableRegisterOverflowError>(
+          RangeType, Range.BaseShaderRegister, Range.RegisterSpace);
----------------
inbelic wrote:

I don't think it makes sense to group all these verifications into the same function and same error message. DXC provided a unique error message for each scenario.

I also think if we split this into 3 validation checks, the functions would be much easier to follow:

1. if `Offset == append && AppendingRegister == unbound` -> implict bound error
2. if `NumDescriptors != unbound && BaseRegister + NumDescriptors - 1 >= ~0u` -> overflow for shader range error
3. Let `Offset = (Offset == append) ? AppendingRegister : Offset`. Then if `Offset + NumDescriptors - 1 >= ~0u` -> overflow for descriptor range error

I would also personally return a `std::optional<uint64_t>` rather than updating `AppendingRegister` as a reference.

https://github.com/llvm/llvm-project/pull/153276