Validator/Dxil version error improvements#3623
Conversation
- Move validator/dxil version checks up-front These should fail first rather than side effects of trying to validate details of a version we don't support. - Improve message for unsupported validator or dxil version These errors are most likely if compiling separately from validation and failing to override the validator version properly, or running on an external validator that doesn't support a newer dxil. - Use dxil version from metadata for DxilModule when loading, rather than just setting it to minimum based on shader model. - Remove TODO from validator messages that shouldn't be there
|
Note: I'd like to get these into release because currently, if you compile normally and then validate separately with DXIL.dll, and the highest version supported by the compiler is not the same as for DXIL.dll, you will get an unhelpful |
pow2clk
left a comment
pow2clk
left a comment
There was a problem hiding this comment.
Looks good! Nice to clean up the errors here as we've run into this a few times.
I do have one suggested addition that might be as easy as I think.
| self.add_valrule("Meta.Used", "All metadata must be used by dxil.") | ||
| self.add_valrule_msg("Meta.Target", "Target triple must be 'dxil-ms-dx'", "Unknown target triple '%0'.") | ||
| self.add_valrule("Meta.WellFormed", "TODO - Metadata must be well-formed in operand count and types.") | ||
| self.add_valrule("Meta.WellFormed", "Metadata must be well-formed in operand count and types.") # TODO: add string arg for what metadata is malformed (this is emitted from a lot of places and provides no context whatsoever) |
There was a problem hiding this comment.
Is it really more complicated than uncommenting this call at 3930 of DxilValidation.cpp?
//ValCtx.EmitMetaError(pNode, ValidationRule::MetaWellFormed);There was a problem hiding this comment.
Oh Hmm. It seems I added that commented out line (#2938). I sure don't remember why I didn't enable it. Probably best to leave it off for now
|
✅ Build DirectXShaderCompiler 1.0.4656 completed (commit 62c2294a1d by @tex3d) |
- Move validator/dxil version checks up-front These should fail first rather than side effects of trying to validate details of a version we don't support. - Improve message for unsupported validator or dxil version These errors are most likely if compiling separately from validation and failing to override the validator version properly, or running on an external validator that doesn't support a newer dxil. - Use dxil version from metadata for DxilModule when loading, rather than just setting it to minimum based on shader model. - Remove TODO from validator messages that shouldn't be there (cherry picked from commit 6244ab8)
- Move validator/dxil version checks up-front These should fail first rather than side effects of trying to validate details of a version we don't support. - Improve message for unsupported validator or dxil version These errors are most likely if compiling separately from validation and failing to override the validator version properly, or running on an external validator that doesn't support a newer dxil. - Use dxil version from metadata for DxilModule when loading, rather than just setting it to minimum based on shader model. - Remove TODO from validator messages that shouldn't be there (cherry picked from commit 6244ab8)
3 participants
These should fail first rather than side effects of trying to validate
details of a version we don't support.
These errors are most likely if compiling separately from validation
and failing to override the validator version properly, or running on
an external validator that doesn't support a newer dxil.
rather than just setting it to minimum based on shader model.