If I understand correctly, the biggest point of contention is that henri doesn't want the shader runners to lose control over the compilation:
It should be entirely up to individual runners to decide which configurations to run, and ideally this would be entirely opaque to the parser.
Because we may eventually want to support runners that don't use vkd3d-shader or the native HLSL compiler, or we may want to extend the shader_runner to other input formats.
I don't *think* this is what Henri was getting at. Nor do I think it's a particularly salient point: *if* we're going to test other input formats than HLSL we're going to necessarily need to invent new paths to do it.
The interesting variables in question right now (shader model, d3dcompiler version) inhere to HLSL and don't inhere to any 3D API. For any other variables the opposite is true I think.
FWIW, wrt asm, I think the right thing to do there is actually to compare the output directly, since there really should be no difference. I have some tests I've been working on along those lines.
- But also, each runner should be in charge of the compilation (we need a shader_runner->compile function pointer field), it receives the shader model and the compilation parameters from the tests but it can also add additional parameters, or even decide to use another compiler altogether, the point is that it should aim to give the same hr as the native compiler would (otherwise it should be marked as "todo"), but most of them would rely on a default function that uses vkd3d-shader (or d3dcompiler_47.dll, if available) for SM<6 and dxcompiler for SM6 (if available).
- We reroute the compilation tests in the [shader] directives to the runners, using the shader_runner->compile instead of calling the common shader_runner.c:compile_shader(), I don't see the need now of having these separated from the runners after this.
Yeah, we could use some deduplication there probably.
- We also pass the shader model and compilation parameters to runner->check_requirements and we expect it to indicate whether it cannot run, it can run, or it can only compile. We use the latter to extend the Vulkan runner to SM1 but just compilation and not execution.
I don't really like that, it doesn't seem very declarative. I'd prefer to add a specific "compile-only" runner or pseudo-runner like this(?) patch series did, but we're also two short steps away from actual sm1 support so I don't think we need to bother anymore.
- We add a `runner->tag` string field to the runner's, which would be `vk` for vulkan, `gl` for opengl, etc.
- We extend the todo() syntax so that both shader model ranges and runner tags can be specified, like `todo(vk,sm>=4,sm<6)`, we also allow multiple todo() markers so that several qualifiers in the same marker work as AND and several markers work as OR, e.g. `todo(vk,sm>=4,sm<6) todo(gl)` if we want to specify that the test doesn't work with gl yet and with vulkan only in SM4.
- We add the following boolean fields to the runner `runner->ignore_compilation_todos`. When we are using a native compiler, we can set this flag true so that we ignore all the `todo` tags for [shader] tests.
- Similarly, we add `runner->ignore_execution_todos` so that, if `true` all the `todo` tags are ignored in the [test] tests. This would be useful for the native runners (d3d9.dll, d3d11.dll, and d3d12.dll), alternatively, we could make the `todo()` syntax not include them by default, but I think this hardcoding can be avoided with this flag.
- Similarly we extend the fail() syntax, but in this case it only needs shader model ranges.
Some of this can probably be hardcoded without needing to add unnecessary flexibility, but I'll wait until I read the patches.