update resource range analysis to use retained source loc

Author: inbelic

clang/include/clang/Sema/SemaHLSL.h

@@ -139,7 +139,9 @@ class SemaHLSL : public SemaBase {
                                ArrayRef<hlsl::RootSignatureElement> Elements);
 
   // Returns true when D is invalid and a diagnostic was produced
-  bool handleRootSignatureDecl(HLSLRootSignatureDecl *D, SourceLocation Loc);
+  bool
+  handleRootSignatureElements(ArrayRef<hlsl::RootSignatureElement> Elements,
+                              SourceLocation Loc);
   void handleRootSignatureAttr(Decl *D, const ParsedAttr &AL);
   void handleNumThreadsAttr(Decl *D, const ParsedAttr &AL);
   void handleWaveSizeAttr(Decl *D, const ParsedAttr &AL);

clang/lib/Sema/SemaHLSL.cpp

@@ -28,6 +28,7 @@
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/Specifiers.h"
 #include "clang/Basic/TargetInfo.h"
+#include "clang/Parse/ParseHLSLRootSignature.h"
 #include "clang/Sema/Initialization.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/ParsedAttr.h"
@@ -1066,6 +1067,9 @@ void SemaHLSL::ActOnFinishRootSignatureDecl(
     SourceLocation Loc, IdentifierInfo *DeclIdent,
     ArrayRef<hlsl::RootSignatureElement> RootElements) {
 
+  if (handleRootSignatureElements(RootElements, Loc))
+    return;
+
   SmallVector<llvm::hlsl::rootsig::RootElement> Elements;
   for (auto &RootSigElement : RootElements)
     Elements.push_back(RootSigElement.getElement());
@@ -1074,21 +1078,24 @@ void SemaHLSL::ActOnFinishRootSignatureDecl(
       SemaRef.getASTContext(), /*DeclContext=*/SemaRef.CurContext, Loc,
       DeclIdent, SemaRef.getLangOpts().HLSLRootSigVer, Elements);
 
-  if (handleRootSignatureDecl(SignatureDecl, Loc))
-    return;
-
   SignatureDecl->setImplicit();
   SemaRef.PushOnScopeChains(SignatureDecl, SemaRef.getCurScope());
 }
 
-bool SemaHLSL::handleRootSignatureDecl(HLSLRootSignatureDecl *D,
-                                       SourceLocation Loc) {
+bool SemaHLSL::handleRootSignatureElements(
+    ArrayRef<hlsl::RootSignatureElement> Elements, SourceLocation Loc) {
   using RangeInfo = llvm::hlsl::rootsig::RangeInfo;
   using OverlappingRanges = llvm::hlsl::rootsig::OverlappingRanges;
 
+  // Introduce a mapping from the collected RangeInfos back to the
+  // RootSignatureElement that will retain its diagnostics info
+  llvm::DenseMap<size_t, const hlsl::RootSignatureElement *> InfoIndexMap;
+  size_t InfoIndex = 0;
+
   // 1. Collect RangeInfos
   llvm::SmallVector<RangeInfo> Infos;
-  for (const llvm::hlsl::rootsig::RootElement &Elem : D->getRootElements()) {
+  for (const hlsl::RootSignatureElement &RootSigElem : Elements) {
+    const llvm::hlsl::rootsig::RootElement &Elem = RootSigElem.getElement();
     if (const auto *Descriptor =
             std::get_if<llvm::hlsl::rootsig::RootDescriptor>(&Elem)) {
       RangeInfo Info;
@@ -1099,6 +1106,9 @@ bool SemaHLSL::handleRootSignatureDecl(HLSLRootSignatureDecl *D,
           llvm::dxil::ResourceClass(llvm::to_underlying(Descriptor->Type));
       Info.Space = Descriptor->Space;
       Info.Visibility = Descriptor->Visibility;
+
+      Info.Index = InfoIndex++;
+      InfoIndexMap[Info.Index] = &RootSigElem;
       Infos.push_back(Info);
     } else if (const auto *Constants =
                    std::get_if<llvm::hlsl::rootsig::RootConstants>(&Elem)) {
@@ -1109,6 +1119,9 @@ bool SemaHLSL::handleRootSignatureDecl(HLSLRootSignatureDecl *D,
       Info.Class = llvm::dxil::ResourceClass::CBuffer;
       Info.Space = Constants->Space;
       Info.Visibility = Constants->Visibility;
+
+      Info.Index = InfoIndex++;
+      InfoIndexMap[Info.Index] = &RootSigElem;
       Infos.push_back(Info);
     } else if (const auto *Sampler =
                    std::get_if<llvm::hlsl::rootsig::StaticSampler>(&Elem)) {
@@ -1119,6 +1132,9 @@ bool SemaHLSL::handleRootSignatureDecl(HLSLRootSignatureDecl *D,
       Info.Class = llvm::dxil::ResourceClass::Sampler;
       Info.Space = Sampler->Space;
       Info.Visibility = Sampler->Visibility;
+
+      Info.Index = InfoIndex++;
+      InfoIndexMap[Info.Index] = &RootSigElem;
       Infos.push_back(Info);
     } else if (const auto *Clause =
                    std::get_if<llvm::hlsl::rootsig::DescriptorTableClause>(
@@ -1133,7 +1149,10 @@ bool SemaHLSL::handleRootSignatureDecl(HLSLRootSignatureDecl *D,
 
       Info.Class = Clause->Type;
       Info.Space = Clause->Space;
+
       // Note: Clause does not hold the visibility this will need to
+      Info.Index = InfoIndex++;
+      InfoIndexMap[Info.Index] = &RootSigElem;
       Infos.push_back(Info);
     } else if (const auto *Table =
                    std::get_if<llvm::hlsl::rootsig::DescriptorTable>(&Elem)) {
@@ -1150,13 +1169,15 @@ bool SemaHLSL::handleRootSignatureDecl(HLSLRootSignatureDecl *D,
   }
 
   // Helper to report diagnostics
-  auto ReportOverlap = [this, Loc](OverlappingRanges Overlap) {
+  auto ReportOverlap = [this, &InfoIndexMap](OverlappingRanges Overlap) {
     const RangeInfo *Info = Overlap.A;
     const RangeInfo *OInfo = Overlap.B;
     auto CommonVis = Info->Visibility == llvm::dxbc::ShaderVisibility::All
                          ? OInfo->Visibility
                          : Info->Visibility;
-    this->Diag(Loc, diag::err_hlsl_resource_range_overlap)
+    const hlsl::RootSignatureElement *Elem = InfoIndexMap.at(Info->Index);
+    SourceLocation InfoLoc = Elem->getLocation();
+    this->Diag(InfoLoc, 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)

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

@@ -50,6 +50,9 @@ struct RangeInfo {
   llvm::dxil::ResourceClass Class;
   uint32_t Space;
   llvm::dxbc::ShaderVisibility Visibility;
+
+  // The index retains its original position before being sorted by group.
+  size_t Index;
 };
 
 class ResourceRange {