microsoft / microsoft/DirectXShaderCompiler
[Validation] Organize usage of GetResourceFromHandle and GetResourceFromVal calls in validation
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 3.7k
- Forks
- 900
- Avg merge
- 2d 11h
- Merged PRs (30d)
- 44
Description
Is your feature request related to a problem? Please describe.
For invalid resource handles, if/when there is validation of the operation itself, usually this will require retrieval of the handle, often using GetResourceFromHandle again. This means any errors emitted there will now be emitted twice for these operations.
The current way that some things flow through to GetResourceFromHandle and emit errors, while some others use GetResourceFromVal which emits no errors, is currently a mess.
Describe the solution you'd like
Handle validation should be up-front, like this, to guarantee that it always emits a consistent error for invalid handles. Then, when looking up handle properties in the individual operation validation cases, those should be using GetResourceFromVal() and IsValid() to skip the ResourceProperties-based checks on invalid handles, to avoid spurious additional errors when an error will have already been emitted for an invalid handle.
The distinction between GetResourceFromHandle and GetResourceFromVal name-wise and functionality-wise doesn't make sense. The first should be changed into a validation-only check with a new name, such as ValidateResourceHandle(). Then this would be called under this basic handle validation pass only. Other uses of GetResourceFromHandle would be changed to use GetResourceFromVal instead. Then all uses of GetResourceFromVal should be checked to make sure any uses of those resource properties for additional validation are placed under control flow guarded by IsValid() to avoid the additional spurious errors.
Note that GetResourceFromHandle is used in functions for getting specific properties (GetSamplerKind, GetResourceKindAndCompTy, GetCBufSize). Those functions should probably DXASSERT if the handle is invalid, since callers should guard against that before using them. Then the callers need new code to call GetResourceFromVal and IsValid() to guard those code paths. Note that ValidateSampleInst and ValidateGather could share the code that calls GetSamplerKind (and checks for valid handle) with a tiny refactor.
Additional context
Full context can be seen here:
https://github.com/microsoft/DirectXShaderCompiler/pull/5399#discussion_r1269919402
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the linked PR discussion and locate GetResourceFromHandle, GetResourceFromVal, IsValid, and the callers named in the issue, including ValidateSampleInst, ValidateGather, GetSamplerKind, GetResourceKindAndCompTy, and GetCBufSize. Trace handle validation and property checks across those entry points. Done means invalid handles produce one consistent validation error without additional spurious errors.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- compilers
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100