|
| 1 | +<!-- {% raw %} --> |
| 2 | + |
| 3 | +# Feature name |
| 4 | + |
| 5 | +## Instructions |
| 6 | + |
| 7 | +* Proposal: [INF-NNNN](INF-NNNN-validator-backcompat-testing.md) |
| 8 | +* Author(s): [Damyan Pepper](https://github.com/damyanp) |
| 9 | +* Sponsor: [Damyan Pepper](https://github.com/damyanp) |
| 10 | +* Status: **Under Consideration** |
| 11 | +* Impacted Project(s): DXC |
| 12 | + |
| 13 | +## Introduction |
| 14 | + |
| 15 | +We propose moving the DXC validator backwards compatability tests from a |
| 16 | +private, internal, repo to the public one, cleaning up the mechanisms that |
| 17 | +enable this as we go. |
| 18 | + |
| 19 | +## Motivation |
| 20 | + |
| 21 | +Currently, internal builds of DXC perform various tests that involve running DXC |
| 22 | +against older versions of the validator (dxil.dll). This is to ensure that |
| 23 | +modifications to DXC (including modifications to the validator) don't break |
| 24 | +backwards compatability with previously released versions. In the past, the |
| 25 | +validator was closed source and so these tests were run from internal builds. |
| 26 | + |
| 27 | +We've recently [open sourced the validator](./INF-0004-validator-hashing.md) and |
| 28 | +as part of this work we want to make DXC default to using the built-in, |
| 29 | +"internal", validator rather than one loaded from a dll, and this breaks these |
| 30 | +tests. |
| 31 | + |
| 32 | +While we're fixing these tests, we're going to take this opportunity to move the |
| 33 | +back-compat testing into the public repo and simplify the infrastructure |
| 34 | +required to run them. |
| 35 | + |
| 36 | + |
| 37 | +## Proposed solution |
| 38 | + |
| 39 | +### How it works currently |
| 40 | + |
| 41 | +These tests only run for Windows x64 builds. The internal build pipeline runs |
| 42 | +`mm test --validator-backcompat-all`. This script has a list of old dxil.dll |
| 43 | +versions to test. The DLL's themselves are checked into the repo. Before running |
| 44 | +each test the dxil.dll is copied into directory next to dxc.exe, with the |
| 45 | +intention that this is the one that DXC will pick up. (Note: in the past we've |
| 46 | +accidentally packaged up and released one of these old dxil.dll.) |
| 47 | + |
| 48 | +The tests themselves are run via lit. |
| 49 | + |
| 50 | +Most of the tests assume that the external validator is used, although a small |
| 51 | +number explicitly set `-select-validator internal` so that when that test runs |
| 52 | +it uses the internal validator - effectively making the test pointless in the |
| 53 | +back compat suite. |
| 54 | + |
| 55 | +There are also TAEF tests that invoke the compiler via the API. We need to |
| 56 | +ensure that these continue to work. |
| 57 | + |
| 58 | +### Proposed Changes |
| 59 | + |
| 60 | +The tests run via binaries downloaded from github releases and are integrated |
| 61 | +into the cmake build system so they can be run from a single target. |
| 62 | + |
| 63 | + |
| 64 | +## Detailed design |
| 65 | + |
| 66 | +These tests are integrated into the build system and can be invoked via a cmake |
| 67 | +target. eg `cmake --build . --target check-validator-backwards-compatability`. |
| 68 | +This target is not run by default, since most developers won't want to download |
| 69 | +the binaries and run these tests. |
| 70 | + |
| 71 | + |
| 72 | +### Binaries |
| 73 | + |
| 74 | +Rather than check the binaries in to source control, we can download them from |
| 75 | +github.com as zip files. `ExternalProject_Add` can be used to do this. |
| 76 | + |
| 77 | +### Specifying which DXIL.dll to use |
| 78 | + |
| 79 | +The `-select-validator` option will be deprecated. As this is not documented, |
| 80 | +it should be ok to just remove it entirely. |
| 81 | + |
| 82 | +A new option, `-dxil-dll-path`, can be used to specify the full path to dxil.dll |
| 83 | +to use. DXC will also be modified to check for a new environment variable, |
| 84 | +`DXC_DXIL_DLL_PATH`, that also specifies the full path to the dll. Example |
| 85 | +path: `c:\dxc\build\old-releases\1.7.123\bin\x64\dxil.dll`. |
| 86 | + |
| 87 | +The command-line option takes precedance over the environment variable. Setting |
| 88 | +the path to an empty string forces the internal validator. |
| 89 | + |
| 90 | +If a dll path is specified and isn't found then DXC will fail with an error. |
| 91 | + |
| 92 | +Note that the environment variable is used for both API as well as command-line |
| 93 | +invocation of the compiler. |
| 94 | + |
| 95 | +### Invoking the tests |
| 96 | + |
| 97 | +The tests are invoked via `add_custom_command`, using `cmake -E env` to set the |
| 98 | +`DXC_DXIL_DLL_PATH` variable. |
| 99 | + |
| 100 | + |
| 101 | + |
| 102 | + |
| 103 | + |
| 104 | + |
| 105 | +## Acknowledgments (Optional) |
| 106 | + |
| 107 | +* [Josh Batista](https://github.com/bob80905) |
| 108 | +* [Tex Riddell](https://github.com/tex3d) |
| 109 | +* [Chris Bieneman](https://github.com/llvm-beanz) |
| 110 | + |
| 111 | +<!-- {% endraw %} --> |
0 commit comments