[DirectX] Adding missing descriptor table validations

clang/lib/Sema/SemaHLSL.cpp

@@ -1394,9 +1394,8 @@ bool SemaHLSL::handleRootSignatureElements(
           Diag(Loc, diag::err_hlsl_offset_overflow) << Offset << RangeBound;
         }
 
-        Offset = RangeBound == llvm::hlsl::rootsig::NumDescriptorsUnbounded
-                     ? uint32_t(RangeBound)
-                     : uint32_t(RangeBound + 1);
+        Offset = llvm::hlsl::rootsig::updateOngoingOffset(
+            Offset, Clause->NumDescriptors, Clause->Offset);
 
         // Compute the register bounds and track resource binding
         uint32_t LowerBound(Clause->Reg.Number);

llvm/include/llvm/Frontend/HLSL/RootSignatureMetadata.h

@@ -48,6 +48,93 @@ class RootSignatureValidationError
   }
 };
 
+class OffsetOverflowError : public ErrorInfo<OffsetOverflowError> {
+public:
+  static char ID;
+  dxil::ResourceClass Type;
+  uint32_t Register;
+  uint32_t Space;
+
+  OffsetOverflowError(dxil::ResourceClass Type, uint32_t Register,
+                      uint32_t Space)
+      : Type(Type), Register(Register), Space(Space) {}
+
+  void log(raw_ostream &OS) const override {
+    OS << "Cannot append range with implicit lower bound after an unbounded "
+          "range "
+       << getResourceClassName(Type) << "(register=" << Register
+       << ", space=" << Space << ").";
+  }
+
+  std::error_code convertToErrorCode() const override {
+    return llvm::inconvertibleErrorCode();
+  }
+};
+
+class ShaderRegisterOverflowError
+    : public ErrorInfo<ShaderRegisterOverflowError> {
+public:
+  static char ID;
+  dxil::ResourceClass Type;
+  uint32_t Register;
+  uint32_t Space;
+
+  ShaderRegisterOverflowError(dxil::ResourceClass Type, uint32_t Register,
+                              uint32_t Space)
+      : Type(Type), Register(Register), Space(Space) {}
+
+  void log(raw_ostream &OS) const override {
+    OS << "Overflow for shader register range: " << getResourceClassName(Type)
+       << "(register=" << Register << ", space=" << Space << ").";
+  }
+
+  std::error_code convertToErrorCode() const override {
+    return llvm::inconvertibleErrorCode();
+  }
+};
+
+class DescriptorRangeOverflowError
+    : public ErrorInfo<DescriptorRangeOverflowError> {
+public:
+  static char ID;
+  dxil::ResourceClass Type;
+  uint32_t Register;
+  uint32_t Space;
+
+  DescriptorRangeOverflowError(dxil::ResourceClass Type, uint32_t Register,
+                               uint32_t Space)
+      : Type(Type), Register(Register), Space(Space) {}
+
+  void log(raw_ostream &OS) const override {
+    OS << "Overflow for descriptor range: " << getResourceClassName(Type)
+       << "(register=" << Register << ", space=" << Space << ").";
+  }
+
+  std::error_code convertToErrorCode() const override {
+    return llvm::inconvertibleErrorCode();
+  }
+};
+
+class TableSamplerMixinError : public ErrorInfo<TableSamplerMixinError> {
+public:
+  static char ID;
+  dxil::ResourceClass Type;
+  uint32_t Location;
+
+  TableSamplerMixinError(dxil::ResourceClass Type, uint32_t Location)
+      : Type(Type), Location(Location) {}
+
+  void log(raw_ostream &OS) const override {
+    OS << "Samplers cannot be mixed with other "
+       << "resource types in a descriptor table, " << getResourceClassName(Type)
+       << "(location=" << Location << ")";
+  }
+
+  std::error_code convertToErrorCode() const override {
+    return llvm::inconvertibleErrorCode();
+  }
+};
+
 class GenericRSMetadataError : public ErrorInfo<GenericRSMetadataError> {
 public:
   LLVM_ABI static char ID;

llvm/include/llvm/Frontend/HLSL/RootSignatureValidations.h

@@ -41,6 +41,8 @@ LLVM_ABI bool verifyLOD(float LOD);
 LLVM_ABI bool verifyBoundOffset(uint32_t Offset);
 LLVM_ABI bool verifyNoOverflowedOffset(uint64_t Offset);
 LLVM_ABI uint64_t computeRangeBound(uint32_t Offset, uint32_t Size);
+LLVM_ABI uint64_t updateOngoingOffset(uint64_t CurOffset,
+                                      uint64_t NumDescriptors, uint64_t Offset);
 
 } // namespace rootsig
 } // namespace hlsl

llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp

@@ -27,6 +27,11 @@ namespace rootsig {
 char GenericRSMetadataError::ID;
 char InvalidRSMetadataFormat::ID;
 char InvalidRSMetadataValue::ID;
+char TableSamplerMixinError::ID;
+char ShaderRegisterOverflowError::ID;
+char OffsetOverflowError::ID;
+char DescriptorRangeOverflowError::ID;
+
 template <typename T> char RootSignatureValidationError<T>::ID;
 
 static std::optional<uint32_t> extractMdIntValue(MDNode *Node,
@@ -538,6 +543,54 @@ Error MetadataParser::parseRootSignatureElement(mcdxbc::RootSignatureDesc &RSD,
   llvm_unreachable("Unhandled RootSignatureElementKind enum.");
 }
 
+Error validateDescriptorTableSamplerMixin(mcdxbc::DescriptorTable Table,
+                                          uint32_t Location) {
+  dxil::ResourceClass CurrRC = dxil::ResourceClass::Sampler;
+  for (const mcdxbc::DescriptorRange &Range : Table.Ranges) {
+    if (Range.RangeType == dxil::ResourceClass::Sampler &&
+        CurrRC != dxil::ResourceClass::Sampler)
+      return make_error<TableSamplerMixinError>(CurrRC, Location);
+    CurrRC = Range.RangeType;
+  }
+  return Error::success();
+}
+
+Error validateDescriptorTableRegisterOverflow(mcdxbc::DescriptorTable Table,
+                                              uint32_t Location) {
+  uint64_t Offset = 0;
+
+  for (const mcdxbc::DescriptorRange &Range : Table.Ranges) {
+    // Validation of NumDescriptors should have happened by this point.
+    if (Range.NumDescriptors == 0)
+      continue;
+
+    if (Range.OffsetInDescriptorsFromTableStart != DescriptorTableOffsetAppend)
+      Offset = Range.OffsetInDescriptorsFromTableStart;
+
+    if (!verifyNoOverflowedOffset(Offset))
+      return make_error<OffsetOverflowError>(
+          Range.RangeType, Range.BaseShaderRegister, Range.RegisterSpace);
+
+    const uint64_t RangeBound = llvm::hlsl::rootsig::computeRangeBound(
+        Range.BaseShaderRegister, Range.NumDescriptors);
+
+    if (!verifyNoOverflowedOffset(RangeBound))
+      return make_error<ShaderRegisterOverflowError>(
+          Range.RangeType, Range.BaseShaderRegister, Range.RegisterSpace);
+
+    const uint64_t OffsetBound =
+        llvm::hlsl::rootsig::computeRangeBound(Offset, Range.NumDescriptors);
+
+    if (!verifyNoOverflowedOffset(OffsetBound))
+      return make_error<DescriptorRangeOverflowError>(
+          Range.RangeType, Range.BaseShaderRegister, Range.RegisterSpace);
+    Offset = updateOngoingOffset(Offset, Range.NumDescriptors,
+                                 Range.OffsetInDescriptorsFromTableStart);
+  }
+
+  return Error::success();
+}
+
 Error MetadataParser::validateRootSignature(
     const mcdxbc::RootSignatureDesc &RSD) {
   Error DeferredErrs = Error::success();
@@ -611,6 +664,14 @@ Error MetadataParser::validateRootSignature(
               joinErrors(std::move(DeferredErrs),
                          make_error<RootSignatureValidationError<uint32_t>>(
                              "DescriptorFlag", Range.Flags));
+
+        if (Error Err =
+                validateDescriptorTableSamplerMixin(Table, Info.Location))
+          DeferredErrs = joinErrors(std::move(DeferredErrs), std::move(Err));
+
+        if (Error Err =
+                validateDescriptorTableRegisterOverflow(Table, Info.Location))
+          DeferredErrs = joinErrors(std::move(DeferredErrs), std::move(Err));
       }
       break;
     }

llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp

@@ -141,6 +141,14 @@ uint64_t computeRangeBound(uint32_t Offset, uint32_t Size) {
   return uint64_t(Offset) + uint64_t(Size) - 1;
 }
 
+uint64_t updateOngoingOffset(uint64_t CurOffset, uint64_t NumDescriptors,
+                             uint64_t Offset) {
+  // Append to the current offset if DescriptorTableOffsetAppend is set,
+  // otherwise calculate the new Offset.
+  return Offset == DescriptorTableOffsetAppend ? CurOffset + NumDescriptors
+                                               : Offset + NumDescriptors;
+}
+
 } // namespace rootsig
 } // namespace hlsl
 } // namespace llvm

llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinations.ll

@@ -11,8 +11,9 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
 
 !dx.rootsignatures = !{!2} ; list of function/root signature pairs
 !2 = !{ ptr @main, !3, i32 2 } ; function, root signature
-!3 = !{ !5 } ; list of root signature elements
-!5 = !{ !"DescriptorTable", i32 0, !6, !8, !9, !10, !11, !12, !13, !14, !15, !16, !17, !18, !19, !20 }
+!3 = !{ !5, !21 } ; list of root signature elements
+!5 = !{ !"DescriptorTable", i32 0, !10, !11, !12, !13, !14, !15, !16, !17, !18, !19, !20 }
+!21 = !{ !"DescriptorTable", i32 0, !6, !8, !9 }
 
 ; typedef enum D3D12_DESCRIPTOR_RANGE_FLAGS {
 ;   NONE = 0,
@@ -53,37 +54,20 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
 !20 = !{ !"UAV", i32 5, i32 1, i32 15, i32 5, i32 65540 }
 
 ;DXC:- Name:            RTS0
-;DXC-NEXT:    Size:            380
+;DXC-NEXT:    Size:            400
 ;DXC-NEXT:    RootSignature:
 ;DXC-NEXT:      Version:         2
-;DXC-NEXT:      NumRootParameters: 1
+;DXC-NEXT:      NumRootParameters: 2
 ;DXC-NEXT:      RootParametersOffset: 24
 ;DXC-NEXT:      NumStaticSamplers: 0
-;DXC-NEXT:      StaticSamplersOffset: 380
+;DXC-NEXT:      StaticSamplersOffset: 400
 ;DXC-NEXT:      Parameters:
 ;DXC-NEXT:        - ParameterType:   DescriptorTable
 ;DXC-NEXT:          ShaderVisibility: All
 ;DXC-NEXT:          Table:
-;DXC-NEXT:            NumRanges:       14
-;DXC-NEXT:            RangesOffset:    44
+;DXC-NEXT:            NumRanges:       11
+;DXC-NEXT:            RangesOffset:    56
 ;DXC-NEXT:            Ranges:
-;DXC-NEXT:              - RangeType:       Sampler
-;DXC-NEXT:                NumDescriptors:  1
-;DXC-NEXT:                BaseShaderRegister: 0
-;DXC-NEXT:                RegisterSpace:   1
-;DXC-NEXT:                OffsetInDescriptorsFromTableStart: 4294967295
-;DXC-NEXT:              - RangeType:       Sampler
-;DXC-NEXT:                NumDescriptors:  1
-;DXC-NEXT:                BaseShaderRegister: 0
-;DXC-NEXT:                RegisterSpace:   3
-;DXC-NEXT:                OffsetInDescriptorsFromTableStart: 4294967295
-;DXC-NEXT:                DESCRIPTORS_VOLATILE: true
-;DXC-NEXT:              - RangeType:       Sampler
-;DXC-NEXT:                NumDescriptors:  1
-;DXC-NEXT:                BaseShaderRegister: 0
-;DXC-NEXT:                RegisterSpace:   4
-;DXC-NEXT:                OffsetInDescriptorsFromTableStart: 4294967295
-;DXC-NEXT:                DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS: true
 ;DXC-NEXT:              - RangeType:       SRV
 ;DXC-NEXT:                NumDescriptors:  1
 ;DXC-NEXT:                BaseShaderRegister: 0
@@ -155,3 +139,26 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
 ;DXC-NEXT:                OffsetInDescriptorsFromTableStart: 5
 ;DXC-NEXT:                DATA_STATIC_WHILE_SET_AT_EXECUTE: true
 ;DXC-NEXT:                DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS: true
+;DXC-NEXT:        - ParameterType:   DescriptorTable
+;DXC-NEXT:          ShaderVisibility: All
+;DXC-NEXT:          Table:
+;DXC-NEXT:            NumRanges:       3
+;DXC-NEXT:            RangesOffset:    328
+;DXC-NEXT:            Ranges:
+;DXC-NEXT:              - RangeType:       Sampler
+;DXC-NEXT:                NumDescriptors:  1
+;DXC-NEXT:                BaseShaderRegister: 0
+;DXC-NEXT:                RegisterSpace:   1
+;DXC-NEXT:                OffsetInDescriptorsFromTableStart: 4294967295
+;DXC-NEXT:              - RangeType:       Sampler
+;DXC-NEXT:                NumDescriptors:  1
+;DXC-NEXT:                BaseShaderRegister: 0
+;DXC-NEXT:                RegisterSpace:   3
+;DXC-NEXT:                OffsetInDescriptorsFromTableStart: 4294967295
+;DXC-NEXT:                DESCRIPTORS_VOLATILE: true
+;DXC-NEXT:              - RangeType:       Sampler
+;DXC-NEXT:                NumDescriptors:  1
+;DXC-NEXT:                BaseShaderRegister: 0
+;DXC-NEXT:                RegisterSpace:   4
+;DXC-NEXT:                OffsetInDescriptorsFromTableStart: 4294967295
+;DXC-NEXT:                DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS: true

llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinationsV1.ll

@@ -11,33 +11,40 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
 
 !dx.rootsignatures = !{!2} ; list of function/root signature pairs
 !2 = !{ ptr @main, !3, i32 1 } ; function, root signature
-!3 = !{ !5 } ; list of root signature elements
-!5 = !{ !"DescriptorTable", i32 0, !6, !7 }
+!3 = !{ !5, !8 } ; list of root signature elements
+!5 = !{ !"DescriptorTable", i32 0, !6 }
 !6 = !{ !"Sampler", i32 1, i32 1, i32 0, i32 -1, i32 1 }
+!8 = !{ !"DescriptorTable", i32 0, !7 }
 !7 = !{ !"UAV", i32 5, i32 1, i32 10, i32 5, i32 3 }
 
 
 ; DXC:        - Name:            RTS0
-; DXC-NEXT:     Size:            84
+; DXC-NEXT:     Size:            104
 ; DXC-NEXT:     RootSignature:
 ; DXC-NEXT:       Version:         1
-; DXC-NEXT:       NumRootParameters: 1
+; DXC-NEXT:       NumRootParameters: 2
 ; DXC-NEXT:       RootParametersOffset: 24
 ; DXC-NEXT:       NumStaticSamplers: 0
-; DXC-NEXT:       StaticSamplersOffset: 84
+; DXC-NEXT:       StaticSamplersOffset: 104
 ; DXC-NEXT:       Parameters:
 ; DXC-NEXT:         - ParameterType:   DescriptorTable
 ; DXC-NEXT:           ShaderVisibility: All
 ; DXC-NEXT:           Table:
-; DXC-NEXT:             NumRanges:       2
-; DXC-NEXT:             RangesOffset:    44
+; DXC-NEXT:             NumRanges:       1
+; DXC-NEXT:             RangesOffset:    56
 ; DXC-NEXT:             Ranges:
 ; DXC-NEXT:               - RangeType:       Sampler
 ; DXC-NEXT:                 NumDescriptors:  1
 ; DXC-NEXT:                 BaseShaderRegister: 1
 ; DXC-NEXT:                 RegisterSpace:   0
 ; DXC-NEXT:                 OffsetInDescriptorsFromTableStart: 4294967295
-; DXC-NEXT:               - RangeType:       UAV
+; DXC-NEXT:         - ParameterType:   DescriptorTable
+; DXC-NEXT:           ShaderVisibility: All
+; DXC-NEXT:           Table:
+; DXC-NEXT:             NumRanges:       1
+; DXC-NEXT:             RangesOffset:    84
+; DXC-NEXT:             Ranges:
+; DXC-NEXT:                - RangeType:       UAV
 ; DXC-NEXT:                 NumDescriptors:  5
 ; DXC-NEXT:                 BaseShaderRegister: 1
 ; DXC-NEXT:                 RegisterSpace:   10

llvm/test/CodeGen/DirectX/rootsignature-validation-fail-appending-overflow.ll

@@ -0,0 +1,17 @@
+; RUN: not opt -S -passes='dxil-post-optimization-validation' -mtriple=dxil-pc-shadermodel6.6-compute %s 2>&1 | FileCheck %s
+; This test check if a resource is implicitly overflowing. That means, it is appending a resource after an unbounded range.
+
+; CHECK: error: Cannot append range with implicit lower bound after an unbounded range UAV(register=0, space=0).
+
+define void @CSMain() "hlsl.shader"="compute" {
+entry:
+  ret void
+}
+
+!dx.rootsignatures = !{!0}
+
+!0 = !{ptr @CSMain, !1, i32 2}
+!1 = !{!3}
+!3 = !{!"DescriptorTable", i32 0, !4, !5}
+!4 = !{!"UAV", i32 -1, i32 1, i32 0, i32 2, i32 0}
+!5 = !{!"UAV", i32 1, i32 0, i32 0, i32 -1, i32 0}

llvm/test/CodeGen/DirectX/rootsignature-validation-fail-offset-overflow.ll

@@ -0,0 +1,15 @@
+; RUN: not opt -S -passes='dxil-post-optimization-validation' -mtriple=dxil-pc-shadermodel6.6-compute %s 2>&1 | FileCheck %s
+; CHECK: error: Overflow for descriptor range: UAV(register=0, space=0)
+
+define void @CSMain() "hlsl.shader"="compute" {
+entry:
+  ret void
+}
+
+!dx.rootsignatures = !{!0}
+
+!0 = !{ptr @CSMain, !1, i32 2}
+!1 = !{!3}
+!3 = !{!"DescriptorTable", i32 0, !4, !5}
+!4 = !{!"UAV", i32 100, i32 0, i32 0, i32 4294967294, i32 0}
+!5 = !{!"UAV", i32 1, i32 101, i32 0, i32 10, i32 0}

llvm/test/CodeGen/DirectX/rootsignature-validation-fail-register-overflow.ll

@@ -0,0 +1,13 @@
+; RUN: not opt -S -passes='dxil-post-optimization-validation' -mtriple=dxil-pc-shadermodel6.6-compute %s 2>&1 | FileCheck %s
+; CHECK: error: Overflow for shader register range: UAV(register=4294967295, space=0)
+define void @CSMain() "hlsl.shader"="compute" {
+entry:
+  ret void
+}
+
+!dx.rootsignatures = !{!0}
+
+!0 = !{ptr @CSMain, !1, i32 2}
+!1 = !{!3}
+!3 = !{!"DescriptorTable", i32 0, !4}
+!4 = !{!"UAV", i32 100, i32 4294967295, i32 0, i32 -1, i32 0}