-
Notifications
You must be signed in to change notification settings - Fork 40
First pass at validator backcompat testing proposal #499
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
error. | ||
|
||
When invoked via dxc.exe (ie not through the compiler API), DXC will check for a | ||
new environment variable, `DXC_DXIL_DLL_PATH`, that specifies the full path to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the environment variable's value is also an empty string, will it default to internal validation behavior?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, unless the command-line parameter is specified.
If a dll path is specified and isn't found then the compiler will fail with an | ||
error. | ||
|
||
When invoked via dxc.exe (ie not through the compiler API), DXC will check for a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even if it isn't necessary in the proposal itself, could you describe why the design decision is being made that the environment variable should only be checked when invoked via dxc.exe, not the compiler API?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think generally this is a good outline. I have some concern about not being able to specify loading a DXIL.dll in the same path as the currently used DxCompiler.dll in a relative way.
The `-select-validator` option will be deprecated. As this is not documented, | ||
it should be ok to just remove it entirely. | ||
|
||
A new option, `-dxil-dll-path`, can be used to specify the full path to dxil.dll |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regarding requiring a full path:
Having a way to specify that the DXIL.dll
is in the same path as the dxcompiler.dll
being used would be a useful option.
We already have an option (using -external
and -external-fn
) for loading a different dxcompiler.dll. I do not think this option has such a restriction.
dxc.exe
loads dxcompiler.dll
to do the work without requiring an absolute path.
Related side note: We have discussed revising the way we load dxil.dll
in the past to prevent searching the environment PATH
(only loading it when in the same location as the dxcompiler.dll
loading it).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding here is that there are no scenarios where an end-user of dxc or the API would be using an external dxil.dll validator - they'd always use the one compiled in to dxcompiler.dll
Selecting dxil.dll is purely something we use internally for validating changes to dxc.
Oh yeah, I forgot to comment on something else I had thought of. You can override the environment variable with an explicit argument, but how do you choose the internal validator this way? |
Something like |
We want to make the internal validator the default one, but this impacts how we test that we haven't broken DXIL. This proposal suggests a way to move these tests into the public github and how to modify DXC and the build system to run the tests.