Subject: [PATCH 7/9] tests/shader-runner: Do not apply todo to result probes. Test shaders do not touch upon any unimplemented features in vkd3d-shader and/or vkd3d, and vkd3d-shader should not emit incorrect SPIR-V. If a situation arises where probes are expected to fail, a 'todo' in a 'probe' line should set a different bool flag to be applied to the readback check only.
Removing the todo's is not wrong, but that commit message could have fooled me. What's actually happening is that since commit 26b89cc338c88e766d032f0ee3b6c9995dad02e8, probes are skipped when the corresponding draw fails; there's no point in probing after a failed draw, and if the render target contains uninitialised data it's just going to cause spurious failures. That means these todo's are now redundant.
I don't think that necessarily justifies removing the ability to set a todo on probes though.
+/* Strip __attribute__((ms_abi)) defined in vkd3d_windows.h as dxcompiler does not use it. */ +cpp_quote("#ifdef __x86_64__") +cpp_quote("# undef __stdcall") +cpp_quote("# define __stdcall") +cpp_quote("#endif")
Our actual issue is with STDMETHODCALLTYPE in the generated header, right? We could potentially use #pragma push_macro/pop_macro for this, although perhaps ideally widl would have a way to deal with this.
+/* Prevents a glitch in the ID3D10Blob_Release() macro when applied to an IDxcBlob. */ +#define WIDL_C_INLINE_WRAPPERS
Why is that? Is it because IDxcBlob has a different calling convention than ID3DBlob (we got rid of "ms_abi", after all), and the wrapper papers over the issue by causing the caller to put the blob pointer in the correct register?
@@ -482,9 +499,17 @@ static void parse_test_directive(struct shader_runner *runner, const char *line) int ret; runner->is_todo = false; + runner->is_todo_pipeline = false; if (match_string(line, "todo", &line)) + { + runner->is_todo = runner->minimum_shader_model < SHADER_MODEL_6_0; + runner->is_todo_pipeline = runner->minimum_shader_model >= SHADER_MODEL_6_0; + } + else if (match_string(line, "todo(sm<6)", &line) && runner->minimum_shader_model < SHADER_MODEL_6_0) runner->is_todo = true; + else if (match_string(line, "todo(sm>=6)", &line) && runner->minimum_shader_model >= SHADER_MODEL_6_0) + runner->is_todo_pipeline = true; if (match_string(line, "dispatch", &line)) {
I'm sorry, but it's still a mystery to me what this is trying to achieve. Why is the meaning of "todo" inverted for shader model 6.0+? Why do we need the separate "is_todo_pipeline" flag? Is this somehow related to not applying todo's to HLSL compilation when vkd3d-shader is not used for that? Couldn't we handle that with some minor adjustments to the corresponding "todo_if" conditions?
I suppose that's fine. I wonder though, does this need to be specific to DXIL? If this is useful, I don't think there's a reason we couldn't support dumping disassembly for shaders compiled by d3dcompiler or vkd3d-shader as well?
It's especially useful for DXIL because it's under development. Maybe that could apply to d3dbc too, but I don't think plumbing a shader dump for other binaries would be simple enough to be part of this MR.
Well, sure. You could even make an argument that "--dump-dxil" isn't all that critical to this MR either. It should largely just be a matter of calling D3DDisassemble() though. We don't have an implementation of that for vkd3d-utils yet, but we should, and we can largely just adopt the fairly straightforward implementation from Wine's d3dcompiler.