microsoft / microsoft/DirectXShaderCompiler
[Metadata] Create non-fatal extra metadata logic
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.
In DxilMetadatahelper.cpp, there is a boolean that is set to true when extra metadata is detected, m_bExtraMetadata. However, due to existing logic, the compiler will later set m_bMetadataErrors to true. When validating dxil, in DxilValidation.cpp, the compiler will never reach ValidateMetadata() since HasMetadataErrors() returns true (because m_bMetadataErrors is set to true, because m_bExtraMetadata was set to true).
We should be able to validate metadata even when there is extra metadata, and perhaps change the infrastructure to indicate when there is non-fatal extra metadata, instead of failing fatally and never validating whenever there's extra metadata.
Describe the solution you'd like
One approach is to split m_bExtraMetadata into two, m_bExtraMetadataFatal and m_bExtraMetadataNonFatal. Then, m_bMetadataErrors can be set if m_bExtraMetadataFatal has been set, and m_bMetadataErrors won't depend on the value of m_bExtraMetadataNonFatal. In this way, metadata validation can occur in DxilValidation.cpp even though there is extra metadata.
Describe alternatives you've considered
Another approach is to introduce some extra clarity on diagnostics / error codes in the metadata area in general, and maybe even add diagnostics directly into DxilMetadataHelper.cpp. This may not be the right place for diagnostics, though.
Additional context
This issue was filed because kDxilRangedWaveSizeTag shouldn't be found in Shader Models 6.8, and if it's found in DXIL with an SM of say 6.7, then the m_bExtraMetadata boolean should be set. However, the desired diagnostic isn't being emitted, rather the diagnostic of the encountering of a non-critical metadata error is emitted instead.
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 in DxilMetadatahelper.cpp by tracing how m_bExtraMetadata is set and how metadata errors are recorded, then follow HasMetadataErrors() into DxilValidation.cpp. Compare the fatal and non-fatal cases described for kDxilRangedWaveSizeTag. Done means metadata validation can run when extra metadata is non-fatal while fatal metadata errors still prevent validation, with the intended diagnostic emitted.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- compilers
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100