================
@@ -1381,22 +1382,24 @@ bool SemaHLSL::handleRootSignatureElements(
llvm::hlsl::rootsig::DescriptorTableOffsetAppend) {
// Manually specified the offset
Offset = Clause->Offset;
+ } else if (Unbound) {
+ // Trying to append onto unbound offset
+ Diag(Loc, diag::err_hlsl_appending_onto_unbound);
}
uint64_t RangeBound = llvm::hlsl::rootsig::computeRangeBound(
Offset, Clause->NumDescriptors);
- if (!llvm::hlsl::rootsig::verifyBoundOffset(Offset)) {
- // Trying to append onto unbound offset
- Diag(Loc, diag::err_hlsl_appending_onto_unbound);
- } else if (!llvm::hlsl::rootsig::verifyNoOverflowedOffset(RangeBound))
{
+ if (!llvm::hlsl::rootsig::verifyNoOverflowedOffset(RangeBound)) {
// Upper bound overflows maximum offset
Diag(Loc, diag::err_hlsl_offset_overflow) << Offset << RangeBound;
}
Offset = RangeBound == llvm::hlsl::rootsig::NumDescriptorsUnbounded
? uint32_t(RangeBound)
: uint32_t(RangeBound + 1);
+ Unbound = Clause->NumDescriptors ==
+ llvm::hlsl::rootsig::NumDescriptorsUnbounded;
----------------
tex3d wrote:
The code in this vicinity does seem to have a few minor nits that make it
harder to reason about:
- Offset will be set to the next available offset, unless that's UINT_MAX, so
could there be a bug where a prior not-unbounded Clause ends at UINT_MAX, and
now the next Clause of size 1 appears to fit starting at UINT_MAX? I think
Offset should be a `uint64_t` and always point to the next location. Otherwise,
every value for a `uint32_t Offset` is a valid location for one descriptor to
fit. If that's larger than UINT_MAX, the next one won't fit.
- Inconsistent use of `llvm::hlsl::rootsig::NumDescriptorsUnbounded` vs. `~0u`
when comparing `Clause->NumDescriptors`. We should use the same
`llvm::hlsl::rootsig::NumDescriptorsUnbounded` everywhere.
- You use `llvm::hlsl::rootsig::computeRangeBound` to compute upper bound
elsewhere, but you duplicate that logic to initialize `UpperBound` below. Why
not use `computeRangeBound` for `UpperBound` too?
https://github.com/llvm/llvm-project/pull/159475
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
