[clang] [llvm] [HLSL][Sema] Use hlsl::BindingInfoBuilder instead of RangeInfo. NFC (PR #150634)
Finn Plummer via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 31 09:58:58 PDT 2025
================
@@ -1171,147 +1268,67 @@ bool SemaHLSL::handleRootSignatureElements(
}
}
- using RangeInfo = llvm::hlsl::rootsig::RangeInfo;
- using OverlappingRanges = llvm::hlsl::rootsig::OverlappingRanges;
- using InfoPairT = std::pair<RangeInfo, const hlsl::RootSignatureElement *>;
+ PerVisibilityBindingChecker BindingChecker(this);
+ SmallVector<std::pair<const llvm::hlsl::rootsig::DescriptorTableClause *,
+ const hlsl::RootSignatureElement *>>
+ UnboundClauses;
- // 1. Collect RangeInfos
- llvm::SmallVector<InfoPairT> InfoPairs;
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;
- Info.LowerBound = Descriptor->Reg.Number;
- Info.UpperBound = Info.LowerBound; // use inclusive ranges []
-
- Info.Class =
- llvm::dxil::ResourceClass(llvm::to_underlying(Descriptor->Type));
- Info.Space = Descriptor->Space;
- Info.Visibility = Descriptor->Visibility;
+ uint32_t LowerBound(Descriptor->Reg.Number);
+ uint32_t UpperBound(LowerBound); // inclusive range
- InfoPairs.push_back({Info, &RootSigElem});
+ BindingChecker.trackBinding(
+ Descriptor->Visibility,
+ static_cast<llvm::dxil::ResourceClass>(Descriptor->Type),
+ Descriptor->Space, LowerBound, UpperBound, &RootSigElem);
} 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 []
+ uint32_t LowerBound(Constants->Reg.Number);
+ uint32_t UpperBound(LowerBound); // inclusive range
- Info.Class = llvm::dxil::ResourceClass::CBuffer;
- Info.Space = Constants->Space;
- Info.Visibility = Constants->Visibility;
-
- InfoPairs.push_back({Info, &RootSigElem});
+ BindingChecker.trackBinding(
+ Constants->Visibility, llvm::dxil::ResourceClass::CBuffer,
+ Constants->Space, LowerBound, UpperBound, &RootSigElem);
} 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;
+ uint32_t LowerBound(Sampler->Reg.Number);
+ uint32_t UpperBound(LowerBound); // inclusive range
- InfoPairs.push_back({Info, &RootSigElem});
+ BindingChecker.trackBinding(
+ Sampler->Visibility, llvm::dxil::ResourceClass::Sampler,
+ Sampler->Space, LowerBound, UpperBound, &RootSigElem);
} else if (const auto *Clause =
std::get_if<llvm::hlsl::rootsig::DescriptorTableClause>(
&Elem)) {
- RangeInfo Info;
- Info.LowerBound = Clause->Reg.Number;
- // Relevant error will have already been reported above and needs to be
- // fixed before we can conduct range analysis, so shortcut error return
- if (Clause->NumDescriptors == 0)
- return true;
- 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
- InfoPairs.push_back({Info, &RootSigElem});
+ // We'll process these once we see the table element.
+ UnboundClauses.emplace_back(Clause, &RootSigElem);
} 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 <= InfoPairs.size() && "RootElement");
- // The last Table->NumClauses elements of Infos are the owned Clauses
- // generated RangeInfo
- auto TableInfos =
- MutableArrayRef<InfoPairT>(InfoPairs).take_back(Table->NumClauses);
- for (InfoPairT &Pair : TableInfos)
- Pair.first.Visibility = Table->Visibility;
- }
- }
-
- // 2. Sort with the RangeInfo <operator to prepare it for findOverlapping
- llvm::sort(InfoPairs,
- [](InfoPairT A, InfoPairT B) { return A.first < B.first; });
-
- llvm::SmallVector<RangeInfo> Infos;
- for (const InfoPairT &Pair : InfoPairs)
- Infos.push_back(Pair.first);
-
- // Helpers to report diagnostics
- uint32_t DuplicateCounter = 0;
- using ElemPair = std::pair<const hlsl::RootSignatureElement *,
- const hlsl::RootSignatureElement *>;
- auto GetElemPair = [&Infos, &InfoPairs, &DuplicateCounter](
- OverlappingRanges Overlap) -> ElemPair {
- // Given we sorted the InfoPairs (and by implication) Infos, and,
- // that Overlap.B is the item retrieved from the ResourceRange. Then it is
- // guarenteed that Overlap.B <= Overlap.A.
- //
- // So we will find Overlap.B first and then continue to find Overlap.A
- // after
- auto InfoB = std::lower_bound(Infos.begin(), Infos.end(), *Overlap.B);
- auto DistB = std::distance(Infos.begin(), InfoB);
- auto PairB = InfoPairs.begin();
- std::advance(PairB, DistB);
-
- auto InfoA = std::lower_bound(InfoB, Infos.end(), *Overlap.A);
- // Similarily, from the property that we have sorted the RangeInfos,
- // all duplicates will be processed one after the other. So
- // DuplicateCounter can be re-used for each set of duplicates we
- // encounter as we handle incoming errors
- DuplicateCounter = InfoA == InfoB ? DuplicateCounter + 1 : 0;
- auto DistA = std::distance(InfoB, InfoA) + DuplicateCounter;
- auto PairA = PairB;
- std::advance(PairA, DistA);
-
- return {PairA->second, PairB->second};
- };
-
- auto ReportOverlap = [this, &GetElemPair](OverlappingRanges Overlap) {
- auto Pair = GetElemPair(Overlap);
- const RangeInfo *Info = Overlap.A;
- const hlsl::RootSignatureElement *Elem = Pair.first;
- const RangeInfo *OInfo = Overlap.B;
-
- auto CommonVis = Info->Visibility == llvm::dxbc::ShaderVisibility::All
- ? OInfo->Visibility
- : Info->Visibility;
- this->Diag(Elem->getLocation(), 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
- << /*unbounded=*/(OInfo->UpperBound == RangeInfo::Unbounded)
- << OInfo->UpperBound << Info->Space << CommonVis;
-
- const hlsl::RootSignatureElement *OElem = Pair.second;
- this->Diag(OElem->getLocation(), diag::note_hlsl_resource_range_here);
- };
-
- // 3. Invoke find overlapping ranges
- llvm::SmallVector<OverlappingRanges> Overlaps =
- llvm::hlsl::rootsig::findOverlappingRanges(Infos);
- for (OverlappingRanges Overlap : Overlaps)
- ReportOverlap(Overlap);
+ assert(UnboundClauses.size() == Table->NumClauses &&
+ "Wrong number of clauses in table?");
----------------
inbelic wrote:
```suggestion
"Number of preceding table elements does not match the table's number of clauses");
```
https://github.com/llvm/llvm-project/pull/150634
More information about the llvm-commits
mailing list