microsoft / microsoft/DirectXShaderCompiler

[Validation] Introduce validation diagnostics in DxilMetadataHelper.cpp

Open
#6,239 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

enhancement validation
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.
DxilMetadataHelper.cpp is used in assembling the DXIL Container, which is a step that happens before DXIL Validation. There are several validation rules that should be tested in DXILValidation.cpp, but these validation rules are assumed to hold in parsing and constructing the DXILContainer in DxilMetadataHelper.cpp. DxilMetadataHelper.cpp has no validation diagnostics, it simply assumes these validation rules are true, and if they aren't, the validator fatally crashes without any helpful diagnostics.

Describe the solution you'd like
We need to introduce a way to emit validation diagnostics in DxilMetadataHelper.cpp, because we need to test our validation rules.

Describe alternatives you've considered
Another alternative is to redesign all of validation so that some DXIL validation happens before container assembly, then container assembly happens, then the rest of the DXIL Validation (if any depends on the container being assembled) gets tested.

Additional context
This issue was inspired in https://github.com/microsoft/DirectXShaderCompiler/issues/6161 because of the inability to test if the wavesize range tag in the DXIL has 3 arguments. In DxilMetadataHelper.cpp, in LoadDxilEntryProperties, when the wavesize range tag is read, it is assumed that there are 3 integer arguments. We cannot assume this, and need validation errors to be emitted if this is not the case. However, we cannot emit these diagnostics because there are no methods by which to emit them in the entire cpp file.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in DxilMetadataHelper.cpp, especially LoadDxilEntryProperties and its handling of the wavesize range tag, then compare the related validation rules in DXILValidation.cpp. Define how malformed metadata should emit validation diagnostics instead of causing a fatal crash, with the three-argument wavesize case from issue 6161 as the concrete completion check.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
compilers, testing-qa
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.