Note that this MR is failing CI.
+enum shader_type +{ + SHADER_TYPE_CS, + SHADER_TYPE_GS, + SHADER_TYPE_HS, + SHADER_TYPE_PS, + SHADER_TYPE_VS, +};
This effectively adds hull and geometry shaders. I suppose that's fine, although it does mean the commit does a bit more than the commit message suggests. But if we're going to add those, why not add domain shaders as well then?
diff --git a/tests/dxcompiler.idl b/tests/dxcompiler.idl new file mode 100644 index 000000000..83b0ed5cd --- /dev/null +++ b/tests/dxcompiler.idl @@ -0,0 +1,155 @@ +import "vkd3d_windows.h"; +#include "vkd3d_unknown.idl"
That's missing our usual copyright header.
+cpp_quote("#ifdef _WIN32") +cpp_quote("#ifndef DXC_API_IMPORT") +cpp_quote("#define DXC_API_IMPORT __declspec(dllimport)") +cpp_quote("#endif") +cpp_quote("#else") +cpp_quote("#ifndef DXC_API_IMPORT") +cpp_quote("#define DXC_API_IMPORT __attribute__ ((visibility (\"default\")))") +cpp_quote("#endif") +cpp_quote("#endif")
I don't think these are used?
+/* 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")
That's going to affect subsequent code as well.
+interface IDxcBlobEncoding : IDxcBlob +{ + HRESULT GetEncoding(BOOL *pKnown, UINT32 *pCodePage); +}
That's not our usual convention for parameter names. Affects IDxcIncludeHandler, IDxcOperationResult, IDxcResult, and IDxcCompiler3 as well.
+typedef struct DxcDefine +{ + const WCHAR *Name; + const WCHAR *Value; +} DxcDefine; +
This seems to be unused.
@@ -478,13 +493,22 @@ static void read_uint4(const char **line, struct uvec4 *v) static void parse_test_directive(struct shader_runner *runner, const char *line) { + bool use_dxcompiler = runner->minimum_shader_model >= SHADER_MODEL_6_0; char *rest; int ret; runner->is_todo = false; + runner->is_todo_6 = false; if (match_string(line, "todo", &line)) + { + runner->is_todo = !use_dxcompiler; + runner->is_todo_6 = use_dxcompiler; + } + else if (match_string(line, "todo(sm<6)", &line) && !use_dxcompiler) runner->is_todo = true; + else if (match_string(line, "todo(sm>=6)", &line) && use_dxcompiler) + runner->is_todo_6 = true; if (match_string(line, "dispatch", &line)) {
That just looks confusing to me. What I'd expect here is essentially something like this:
```c if (match_string(line, "todo", &line)) runner->is_todo = true; else if (match_string(line, "todo(sm<6)", &line)) runner->is_todo = runner->minimum_shader_model < SHADER_MODEL_6_0; else if (match_string(line, "todo(sm>=6)", &line)) runner->is_todo = runner->minimum_shader_model >= SHADER_MODEL_6_0; ```
Something similar comes up in a number of other places as well. Fundamentally, I don't think the shader runner should be concerned about where a particular backend runner is using dxcompiler or not.
+#ifdef _WIN32 +static const char dxcompiler_name[] = "dxcompiler.dll"; +#else +static const char dxcompiler_name[] = "libdxcompiler.so"; +#endif
We could probably introduce SONAME_LIBDXCOMPILER for this.
From 5635ad98aa3303a1a7d00080d63afe8f4d683747 Mon Sep 17 00:00:00 2001 From: Conor McCarthy <cmccarthy@codeweavers.com> Date: Thu, 14 Sep 2023 19:42:57 +1000 Subject: [PATCH 12/12] tests/shader-runner: Add a '--dump-dxil' command line switch. Dumps DXIL disassembly when shaders are compiled with dxcompiler.
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?