Merge branch 'validation/check-descriptors-are-bound' into validation/textures-not-bind-root-signatures

Author: joaosaffran

llvm/include/llvm/Frontend/HLSL/HLSLBinding.h

@@ -58,14 +58,16 @@ class BindingInfo {
     }
     // Size == -1 means unbounded array
     LLVM_ABI std::optional<uint32_t> findAvailableBinding(int32_t Size);
-    LLVM_ABI bool isBound(BindingRange B);
+    LLVM_ABI bool isBound(const BindingRange &Range) const;
   };
 
   struct BindingSpaces {
     dxil::ResourceClass RC;
     llvm::SmallVector<RegisterSpace> Spaces;
     BindingSpaces(dxil::ResourceClass RC) : RC(RC) {}
     LLVM_ABI RegisterSpace &getOrInsertSpace(uint32_t Space);
+    LLVM_ABI std::optional<const BindingInfo::RegisterSpace *>
+    contains(uint32_t Space) const;
   };
 
 private:
@@ -97,7 +99,8 @@ class BindingInfo {
   LLVM_ABI std::optional<uint32_t>
   findAvailableBinding(dxil::ResourceClass RC, uint32_t Space, int32_t Size);
 
-  LLVM_ABI bool isBound(dxil::ResourceClass RC, uint32_t Space, BindingRange B);
+  LLVM_ABI bool isBound(dxil::ResourceClass RC, uint32_t Space,
+                        const BindingRange &Range) const;
 
   friend class BindingInfoBuilder;
 };

llvm/lib/Frontend/HLSL/HLSLBinding.cpp

@@ -8,6 +8,8 @@
 
 #include "llvm/Frontend/HLSL/HLSLBinding.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Error.h"
+#include <optional>
 
 using namespace llvm;
 using namespace hlsl;
@@ -32,6 +34,18 @@ BindingInfo::BindingSpaces::getOrInsertSpace(uint32_t Space) {
   return Spaces.emplace_back(Space);
 }
 
+std::optional<const BindingInfo::RegisterSpace *>
+BindingInfo::BindingSpaces::contains(uint32_t Space) const {
+  const BindingInfo::RegisterSpace *It = Spaces.begin();
+  for (auto *End = Spaces.end(); It != End; ++It) {
+    if (It->Space == Space)
+      break;
+  }
+  if (It == Spaces.end())
+    return std::nullopt;
+  return It;
+}
+
 std::optional<uint32_t>
 BindingInfo::RegisterSpace::findAvailableBinding(int32_t Size) {
   assert((Size == -1 || Size > 0) && "invalid size");
@@ -66,24 +80,27 @@ BindingInfo::RegisterSpace::findAvailableBinding(int32_t Size) {
   return std::nullopt;
 }
 
-bool BindingInfo::RegisterSpace::isBound(BindingRange B) {
-  BindingRange *It = llvm::lower_bound(
-      FreeRanges, B.LowerBound,
+bool BindingInfo::RegisterSpace::isBound(const BindingRange &Range) const {
+  const BindingRange *It = llvm::lower_bound(
+      FreeRanges, Range.LowerBound,
       [](const BindingRange &R, uint32_t Val) { return R.UpperBound <= Val; });
 
   if (It != FreeRanges.end()) {
     // Check if B is fully contained in the found range
-    if (B.LowerBound >= It->LowerBound && B.UpperBound <= It->UpperBound)
+    if (Range.LowerBound >= It->LowerBound &&
+        Range.UpperBound <= It->UpperBound)
       return false;
   }
   return true;
 }
 
 bool BindingInfo::isBound(dxil::ResourceClass RC, uint32_t Space,
-                          BindingRange B) {
-  BindingSpaces &BS = getBindingSpaces(RC);
-  RegisterSpace &RS = BS.getOrInsertSpace(Space);
-  return RS.isBound(B);
+                          const BindingRange &Range) const {
+  const BindingSpaces &BS = getBindingSpaces(RC);
+  std::optional<const BindingInfo::RegisterSpace *> RS = BS.contains(Space);
+  if (!RS)
+    return false;
+  return RS.value()->isBound(Range);
 }
 
 BindingInfo BindingInfoBuilder::calculateBindingInfo(

llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp

@@ -122,9 +122,8 @@ static void reportInvalidHandleTyError(Module &M, Twine Type,
                                        ResourceInfo::ResourceBinding Binding) {
   SmallString<128> Message;
   raw_svector_ostream OS(Message);
-  OS << "resource " << Type << " at register (space=" << Binding.Space
-     << ", register=" << Binding.LowerBound << ")"
-     << " is bound to a texture or typed buffer.";
+  OS << Type << " at register " << Binding.LowerBound << " and space "
+     << Binding.Space << " is bound to a texture or typed buffer.";
   M.getContext().diagnose(DiagnosticInfoGeneric(Message));
 }
 
@@ -145,31 +144,24 @@ reportOverlappingRegisters(Module &M,
 
 static void
 reportRegNotBound(Module &M, ResourceClass Class,
-                  llvm::dxil::ResourceInfo::ResourceBinding Unbound) {
+                  const llvm::dxil::ResourceInfo::ResourceBinding &Unbound) {
   SmallString<128> Message;
   raw_svector_ostream OS(Message);
-  OS << "register " << getResourceClassName(Class)
-     << " (space=" << Unbound.Space << ", register=" << Unbound.LowerBound
-     << ")"
+  OS << getResourceClassName(Class) << " register " << Unbound.LowerBound
+     << " in space " << Unbound.Space
      << " does not have a binding in the Root Signature";
   M.getContext().diagnose(DiagnosticInfoGeneric(Message));
 }
 
 static void checkInvalidHandleTy(
     Module &M, const llvm::ArrayRef<dxil::ResourceInfo::ResourceBinding> &RDs,
-    const iterator_range<SmallVectorImpl<dxil::ResourceInfo>::iterator>
-        &Resources) {
-  for (auto Res = Resources.begin(), End = Resources.end(); Res != End; Res++) {
-    llvm::dxil::ResourceInfo::ResourceBinding Binding = Res->getBinding();
-    for (const auto &RD : RDs) {
-      if (Binding.overlapsWith(RD)) {
-        TargetExtType *Handle = Res->getHandleTy();
-        auto *TypedBuffer = dyn_cast_or_null<TypedBufferExtType>(Handle);
-        auto *Texture = dyn_cast_or_null<TextureExtType>(Handle);
-
-        if (TypedBuffer != nullptr || Texture != nullptr)
-          reportInvalidHandleTyError(M, Res->getName(), Res->getBinding());
-      }
+    const llvm::dxil::ResourceInfo::ResourceBinding &Binding,
+    const ResourceKind &Kind, const dxil::ResourceClass &RC) {
+  for (const auto &RD : RDs) {
+    if (Binding.overlapsWith(RD)) {
+
+      if (Kind != ResourceKind::RawBuffer)
+        reportInvalidHandleTyError(M, getResourceClassName(RC), Binding);
     }
   }
 }
@@ -239,7 +231,8 @@ getRootDescriptorsBindingInfo(const mcdxbc::RootSignatureDesc &RSD,
 static void validateRootSignature(Module &M,
                                   const mcdxbc::RootSignatureDesc &RSD,
                                   dxil::ModuleMetadataInfo &MMI,
-                                  DXILResourceMap &DRM) {
+                                  DXILResourceMap &DRM,
+                                  DXILResourceTypeMap &DRTM) {
 
   hlsl::BindingInfoBuilder Builder;
   dxbc::ShaderVisibility Visibility = tripleToVisibility(MMI.ShaderProfile);
@@ -305,22 +298,20 @@ static void validateRootSignature(Module &M,
             Builder.findOverlapping(ReportedBinding);
         reportOverlappingRegisters(M, ReportedBinding, Overlaping);
       });
+
   SmallVector<ResourceInfo::ResourceBinding> RDs =
       getRootDescriptorsBindingInfo(RSD, Visibility);
-  for (const auto &ResList :
-       {std::make_pair(ResourceClass::SRV, DRM.srvs()),
-        std::make_pair(ResourceClass::UAV, DRM.uavs()),
-        std::make_pair(ResourceClass::CBuffer, DRM.cbuffers()),
-        std::make_pair(ResourceClass::Sampler, DRM.samplers())}) {
-    for (auto Res : ResList.second) {
-      llvm::dxil::ResourceInfo::ResourceBinding ResBinding = Res.getBinding();
-      llvm::hlsl::BindingInfo::BindingRange ResRange(
-          ResBinding.LowerBound, ResBinding.LowerBound + ResBinding.Size);
-
-      if (!Info.isBound(ResList.first, ResBinding.Space, ResRange))
-        reportRegNotBound(M, ResList.first, ResBinding);
-    }
-    checkInvalidHandleTy(M, RDs, ResList.second);
+
+  for (const ResourceInfo &RI : DRM) {
+    const ResourceInfo::ResourceBinding &Binding = RI.getBinding();
+    dxil::ResourceTypeInfo RTI = DRTM[RI.getHandleTy()];
+    dxil::ResourceClass RC = RTI.getResourceClass();
+
+    if (!Info.isBound(RC, Binding.Space,
+                      {Binding.LowerBound, Binding.LowerBound + Binding.Size}))
+      reportRegNotBound(M, RC, Binding);
+
+    checkInvalidHandleTy(M, RDs, Binding, RTI.getResourceKind(), RC);
   }
 }
 
@@ -335,7 +326,8 @@ getRootSignature(RootSignatureBindingInfo &RSBI,
 static void reportErrors(Module &M, DXILResourceMap &DRM,
                          DXILResourceBindingInfo &DRBI,
                          RootSignatureBindingInfo &RSBI,
-                         dxil::ModuleMetadataInfo &MMI) {
+                         dxil::ModuleMetadataInfo &MMI,
+                         DXILResourceTypeMap &DRTM) {
   if (DRM.hasInvalidCounterDirection())
     reportInvalidDirection(M, DRM);
 
@@ -346,7 +338,7 @@ static void reportErrors(Module &M, DXILResourceMap &DRM,
                                        "DXILResourceImplicitBinding pass");
 
   if (mcdxbc::RootSignatureDesc *RSD = getRootSignature(RSBI, MMI))
-    validateRootSignature(M, *RSD, MMI, DRM);
+    validateRootSignature(M, *RSD, MMI, DRM, DRTM);
 }
 
 PreservedAnalyses
@@ -355,8 +347,9 @@ DXILPostOptimizationValidation::run(Module &M, ModuleAnalysisManager &MAM) {
   DXILResourceBindingInfo &DRBI = MAM.getResult<DXILResourceBindingAnalysis>(M);
   RootSignatureBindingInfo &RSBI = MAM.getResult<RootSignatureAnalysis>(M);
   ModuleMetadataInfo &MMI = MAM.getResult<DXILMetadataAnalysis>(M);
+  DXILResourceTypeMap &DRTM = MAM.getResult<DXILResourceTypeAnalysis>(M);
 
-  reportErrors(M, DRM, DRBI, RSBI, MMI);
+  reportErrors(M, DRM, DRBI, RSBI, MMI, DRTM);
   return PreservedAnalyses::all();
 }
 
@@ -372,8 +365,10 @@ class DXILPostOptimizationValidationLegacy : public ModulePass {
         getAnalysis<RootSignatureAnalysisWrapper>().getRSInfo();
     dxil::ModuleMetadataInfo &MMI =
         getAnalysis<DXILMetadataAnalysisWrapperPass>().getModuleMetadata();
+    DXILResourceTypeMap &DRTM =
+        getAnalysis<DXILResourceTypeWrapperPass>().getResourceTypeMap();
 
-    reportErrors(M, DRM, DRBI, RSBI, MMI);
+    reportErrors(M, DRM, DRBI, RSBI, MMI, DRTM);
     return false;
   }
   StringRef getPassName() const override {
@@ -383,15 +378,18 @@ class DXILPostOptimizationValidationLegacy : public ModulePass {
 
   static char ID; // Pass identification.
   void getAnalysisUsage(llvm::AnalysisUsage &AU) const override {
+
     AU.addRequired<DXILResourceWrapperPass>();
     AU.addRequired<DXILResourceBindingWrapperPass>();
     AU.addRequired<DXILMetadataAnalysisWrapperPass>();
     AU.addRequired<RootSignatureAnalysisWrapper>();
+    AU.addRequired<DXILResourceTypeWrapperPass>();
     AU.addPreserved<DXILResourceWrapperPass>();
     AU.addPreserved<DXILResourceBindingWrapperPass>();
     AU.addPreserved<DXILMetadataAnalysisWrapperPass>();
     AU.addPreserved<ShaderFlagsAnalysisWrapper>();
     AU.addPreserved<RootSignatureAnalysisWrapper>();
+    AU.addPreserved<DXILResourceTypeWrapperPass>();
   }
 };
 char DXILPostOptimizationValidationLegacy::ID = 0;
@@ -404,6 +402,7 @@ INITIALIZE_PASS_DEPENDENCY(DXILResourceTypeWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(DXILResourceWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(DXILMetadataAnalysisWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(RootSignatureAnalysisWrapper)
+INITIALIZE_PASS_DEPENDENCY(DXILResourceTypeWrapperPass)
 INITIALIZE_PASS_END(DXILPostOptimizationValidationLegacy, DEBUG_TYPE,
                     "DXIL Post Optimization Validation", false, false)
 

llvm/test/CodeGen/DirectX/rootsignature-validation-fail-cbv-binding.ll

@@ -1,5 +1,5 @@
 ; RUN: not opt -S -passes='dxil-post-optimization-validation' -mtriple=dxil-pc-shadermodel6.6-compute %s 2>&1 | FileCheck %s
-; CHECK: error: register CBV (space=666, register=2) does not have a binding in the Root Signature
+; CHECK: error: CBV register 2 in space 666 does not have a binding in the Root Signature
 
 %__cblayout_CB = type <{ float }>
 

llvm/test/CodeGen/DirectX/rootsignature-validation-fail-sampler-binding.ll

@@ -1,5 +1,5 @@
 ; RUN: not opt -S -passes='dxil-post-optimization-validation' -mtriple=dxil-pc-shadermodel6.6-compute %s 2>&1 | FileCheck %s
-; CHECK: error: register Sampler (space=2, register=3) does not have a binding in the Root Signature 
+; CHECK: error: Sampler register 3 in space 2 does not have a binding in the Root Signature 
 
 @Smp.str = private unnamed_addr constant [4 x i8] c"Smp\00", align 1
 

llvm/test/CodeGen/DirectX/rootsignature-validation-fail-srv-binding.ll

@@ -1,5 +1,5 @@
 ; RUN: not opt -S -passes='dxil-post-optimization-validation' -mtriple=dxil-pc-shadermodel6.6-compute %s 2>&1 | FileCheck %s
-; CHECK: error: register SRV (space=0, register=0) does not have a binding in the Root Signature
+; CHECK: error: SRV register 0 in space 0 does not have a binding in the Root Signature
 
 @SB.str = private unnamed_addr constant [3 x i8] c"SB\00", align 1
 

llvm/test/CodeGen/DirectX/rootsignature-validation-fail-uav-binding.ll

@@ -1,5 +1,5 @@
 ; RUN: not opt -S -passes='dxil-post-optimization-validation' -mtriple=dxil-pc-shadermodel6.6-compute %s 2>&1 | FileCheck %s
-; CHECK: error: register UAV (space=0, register=4294967294) does not have a binding in the Root Signature
+; CHECK: error: UAV register 4294967294 in space 0 does not have a binding in the Root Signature
 
 @RWB.str = private unnamed_addr constant [4 x i8] c"RWB\00", align 1
 

llvm/test/CodeGen/DirectX/rootsignature-validation-textures-fail.ll

@@ -1,5 +1,5 @@
 ; RUN: not opt -S -passes='dxil-post-optimization-validation' -mtriple=dxil-pc-shadermodel6.6-compute %s 2>&1 | FileCheck %s
-; CHECK: error: resource TB at register (space=0, register=0) is bound to a texture or typed buffer.
+; CHECK: error: UAV at register 0 and space 0 is bound to a texture or typed buffer.
 
 @TB.str = private unnamed_addr constant [3 x i8] c"TB\00", align 1
 

llvm/test/CodeGen/DirectX/rootsignature-validation-typedbuffer-fail.ll

@@ -1,5 +1,5 @@
 ; RUN: not opt -S -passes='dxil-post-optimization-validation' -mtriple=dxil-pc-shadermodel6.6-compute %s 2>&1 | FileCheck %s
-; CHECK: error: resource TB at register (space=0, register=0) is bound to a texture or typed buffer.
+; CHECK: error: UAV at register 0 and space 0 is bound to a texture or typed buffer.
 
 @TB.str = private unnamed_addr constant [3 x i8] c"TB\00", align 1