[PATCH v3 0/4] MR418: tests/shader-runner: finer control over model ranges and [shader] directive execution for backends, part 1.
First part of a series that allows the shader-runner to test SM1 compilation on [shader] directives even if a backend to run these tests is not available and prepare they way for SM1-only tests.
Specifically: - Compile [shader] directives separately from backend execution (on part 1). - Allow specifying more detailed shader ranges for todo(), fail() and notimpl() qualifiers (on part 2). - Always test shader compilation with SM1 profiles (on part 2).
The `minimum_shader_model` and `maximum_shader_model` parameters for run_shader_tests() are interpreted in the following way:
Ask the pertaining backend to run the shader_test file so that each test is executed with the lowest profile it can support within the range of shader models specified, if any.
This allows us to control how many shader models we want to test for each backend, e.g.: ``` run_shader_tests(..., SHADER_MODEL_2_0, SHADER_MODEL_3_0) run_shader_tests(..., SHADER_MODEL_4_0, SHADER_MODEL_5_1) run_shader_tests(..., SHADER_MODEL_6_0, SHADER_MODEL_6_0) ``` versus ``` for (i = SHADER_MODEL_2_0, i <= SHADER_MODEL_6_0, ++i) run_shader_tests(..., i, i); ```
Also, to allow to compile [shader] directives, which are not backend-specific, separately from the [test] directives, which are, the run_shader_tests() function is modified to skip backend specific directives if the shader_runner_ops is NULL.
Following patches are on my [master6i](https://gitlab.winehq.org/fcasas/vkd3d/-/commits/master6i) branch.
-- v3: tests/shader-runner: Compile [shader] directives separately from backend execution. tests/shader-runner: Execute runner for many shader model ranges if it supports it. tests/shader-runner: Discern between the minimum_shader_model and the selected_shader_model.
From: Francisco Casas fcasas@codeweavers.com
We need these checks to properly handle tests that require target profiles 3.0 and lower, or tests that require target profile 6.0 and higher.
Since all backeds require an ops->check_requirements now, expect it to always be defined. --- tests/shader_runner.c | 3 ++- tests/shader_runner.h | 4 ++-- tests/shader_runner_d3d11.c | 13 +++++++++++++ tests/shader_runner_d3d12.c | 14 ++++++++++++++ tests/shader_runner_vulkan.c | 13 +++++++++++++ 5 files changed, 44 insertions(+), 3 deletions(-)
diff --git a/tests/shader_runner.c b/tests/shader_runner.c index 59c1408c4..77ee63dd0 100644 --- a/tests/shader_runner.c +++ b/tests/shader_runner.c @@ -1073,8 +1073,9 @@ void run_shader_tests(struct shader_runner *runner, const struct shader_runner_o break;
case STATE_REQUIRE: + assert(runner->ops->check_requirements); if (runner->maximum_shader_model < runner->minimum_shader_model - || (runner->ops->check_requirements && !runner->ops->check_requirements(runner))) + || !runner->ops->check_requirements(runner)) { skip_tests = true; } diff --git a/tests/shader_runner.h b/tests/shader_runner.h index 3efbcf362..f6ef44a71 100644 --- a/tests/shader_runner.h +++ b/tests/shader_runner.h @@ -140,9 +140,9 @@ struct shader_runner
struct shader_runner_ops { - /* Returns false if unable to run the given tests. If NULL, all tests are - * run. */ + /* Returns false if unable to run the given tests. */ bool (*check_requirements)(struct shader_runner *runner); + struct resource *(*create_resource)(struct shader_runner *runner, const struct resource_params *params); void (*destroy_resource)(struct shader_runner *runner, struct resource *resource); bool (*draw)(struct shader_runner *runner, D3D_PRIMITIVE_TOPOLOGY primitive_topology, unsigned int vertex_count); diff --git a/tests/shader_runner_d3d11.c b/tests/shader_runner_d3d11.c index bd9d363ce..7a8f8dac0 100644 --- a/tests/shader_runner_d3d11.c +++ b/tests/shader_runner_d3d11.c @@ -100,6 +100,18 @@ static ID3D10Blob *compile_shader(const struct d3d11_shader_runner *runner, cons return blob; }
+static bool d3d11_runner_check_requirements(struct shader_runner *r) +{ + struct d3d11_shader_runner *runner = d3d11_shader_runner(r); + + if (runner->r.maximum_shader_model < SHADER_MODEL_4_0) + return false; + if (runner->r.minimum_shader_model >= SHADER_MODEL_6_0) + return false; + + return true; +} + static IDXGIAdapter *create_adapter(void) { IDXGIFactory4 *factory4; @@ -724,6 +736,7 @@ static void d3d11_runner_release_readback(struct shader_runner *r, struct resour
static const struct shader_runner_ops d3d11_runner_ops = { + .check_requirements = d3d11_runner_check_requirements, .create_resource = d3d11_runner_create_resource, .destroy_resource = d3d11_runner_destroy_resource, .dispatch = d3d11_runner_dispatch, diff --git a/tests/shader_runner_d3d12.c b/tests/shader_runner_d3d12.c index 8453617ef..c54cb5384 100644 --- a/tests/shader_runner_d3d12.c +++ b/tests/shader_runner_d3d12.c @@ -96,6 +96,19 @@ static ID3D10Blob *compile_shader(const struct d3d12_shader_runner *runner, cons return blob; }
+static bool d3d12_runner_check_requirements(struct shader_runner *r) +{ + struct d3d12_shader_runner *runner = d3d12_shader_runner(r); + + if (runner->r.maximum_shader_model < SHADER_MODEL_4_0) + return false; + + if (runner->r.minimum_shader_model >= SHADER_MODEL_6_0 && !runner->dxc_compiler) + return false; + + return true; +} + #define MAX_RESOURCE_DESCRIPTORS (MAX_RESOURCES * 2)
static struct resource *d3d12_runner_create_resource(struct shader_runner *r, const struct resource_params *params) @@ -567,6 +580,7 @@ static void d3d12_runner_release_readback(struct shader_runner *r, struct resour
static const struct shader_runner_ops d3d12_runner_ops = { + .check_requirements = d3d12_runner_check_requirements, .create_resource = d3d12_runner_create_resource, .destroy_resource = d3d12_runner_destroy_resource, .dispatch = d3d12_runner_dispatch, diff --git a/tests/shader_runner_vulkan.c b/tests/shader_runner_vulkan.c index f89b4d624..ec2d1fc9e 100644 --- a/tests/shader_runner_vulkan.c +++ b/tests/shader_runner_vulkan.c @@ -83,6 +83,18 @@ static struct vulkan_shader_runner *vulkan_shader_runner(struct shader_runner *r
#define VK_CALL(f) (runner->f)
+static bool vulkan_runner_check_requirements(struct shader_runner *r) +{ + struct vulkan_shader_runner *runner = vulkan_shader_runner(r); + + if (runner->r.maximum_shader_model < SHADER_MODEL_4_0) + return false; + if (runner->r.minimum_shader_model >= SHADER_MODEL_6_0) + return false; + + return true; +} + static void begin_command_buffer(struct vulkan_shader_runner *runner) { VkCommandBufferBeginInfo buffer_begin_desc = {.sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_BEGIN_INFO}; @@ -1157,6 +1169,7 @@ static void vulkan_runner_release_readback(struct shader_runner *r, struct resou
static const struct shader_runner_ops vulkan_runner_ops = { + .check_requirements = vulkan_runner_check_requirements, .create_resource = vulkan_runner_create_resource, .destroy_resource = vulkan_runner_destroy_resource, .dispatch = vulkan_runner_dispatch,
From: Francisco Casas fcasas@codeweavers.com
In case that the backend selects a shader model that is higher than the minimum in the provided range, it doesn't make sense to check todos(...), fails(...), or notimpl(...) against the minimum.
So the backend must report the selected model for the given range (if any). This is done through the check_requirements() function. --- tests/shader_runner.c | 24 ++++++++++++++---------- tests/shader_runner.h | 6 ++++-- tests/shader_runner_d3d11.c | 8 ++++---- tests/shader_runner_d3d12.c | 12 ++++++------ tests/shader_runner_d3d9.c | 6 ++++-- tests/shader_runner_vulkan.c | 8 ++++---- 6 files changed, 36 insertions(+), 28 deletions(-)
diff --git a/tests/shader_runner.c b/tests/shader_runner.c index 77ee63dd0..9a4840cb0 100644 --- a/tests/shader_runner.c +++ b/tests/shader_runner.c @@ -499,9 +499,9 @@ static void parse_test_directive(struct shader_runner *runner, const char *line) 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; + runner->is_todo = runner->selected_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; + runner->is_todo = runner->selected_shader_model >= SHADER_MODEL_6_0;
if (match_string(line, "dispatch", &line)) { @@ -937,7 +937,7 @@ result_release: static void compile_shader(struct shader_runner *runner, IDxcCompiler3 *dxc_compiler, const char *source, size_t len, enum shader_type type, HRESULT expect) { - bool use_dxcompiler = runner->minimum_shader_model >= SHADER_MODEL_6_0; + bool use_dxcompiler = runner->selected_shader_model >= SHADER_MODEL_6_0; ID3D10Blob *blob = NULL, *errors = NULL; char profile[7]; HRESULT hr; @@ -960,7 +960,7 @@ static void compile_shader(struct shader_runner *runner, IDxcCompiler3 *dxc_comp } else { - sprintf(profile, "%s_%s", shader_type_string(type), shader_models[runner->minimum_shader_model]); + sprintf(profile, "%s_%s", shader_type_string(type), shader_models[runner->selected_shader_model]); hr = D3DCompile(source, len, NULL, NULL, NULL, "main", profile, runner->compile_options, 0, &blob, &errors); } hr = map_unidentified_hrs(hr); @@ -990,7 +990,7 @@ static enum parse_state read_shader_directive(struct shader_runner *runner, enum /* 'todo' is not meaningful when dxcompiler is in use, so it has no '(sm<6) qualifier. */ if (match_directive_substring(src, "todo", &src)) { - if (runner->minimum_shader_model >= SHADER_MODEL_6_0) + if (runner->selected_shader_model >= SHADER_MODEL_6_0) continue; if (state == STATE_SHADER_COMPUTE) state = STATE_SHADER_COMPUTE_TODO; @@ -1005,17 +1005,17 @@ static enum parse_state read_shader_directive(struct shader_runner *runner, enum } else if (match_directive_substring(src, "fail(sm<6)", &src)) { - if (runner->minimum_shader_model < SHADER_MODEL_6_0) + if (runner->selected_shader_model < SHADER_MODEL_6_0) *expect_hr = E_FAIL; } else if (match_directive_substring(src, "fail(sm>=6)", &src)) { - if (runner->minimum_shader_model >= SHADER_MODEL_6_0) + if (runner->selected_shader_model >= SHADER_MODEL_6_0) *expect_hr = E_FAIL; } else if (match_directive_substring(src, "notimpl(sm<6)", &src)) { - if (runner->minimum_shader_model < SHADER_MODEL_6_0) + if (runner->selected_shader_model < SHADER_MODEL_6_0) *expect_hr = E_NOTIMPL; } else @@ -1054,6 +1054,10 @@ void run_shader_tests(struct shader_runner *runner, const struct shader_runner_o runner->ops = ops; runner->minimum_shader_model = minimum_shader_model; runner->maximum_shader_model = maximum_shader_model; + runner->selected_shader_model = minimum_shader_model; + + assert(runner->ops->check_requirements); + skip_tests = !runner->ops->check_requirements(runner, &runner->selected_shader_model);
for (;;) { @@ -1073,9 +1077,8 @@ void run_shader_tests(struct shader_runner *runner, const struct shader_runner_o break;
case STATE_REQUIRE: - assert(runner->ops->check_requirements); if (runner->maximum_shader_model < runner->minimum_shader_model - || !runner->ops->check_requirements(runner)) + || !runner->ops->check_requirements(runner, &runner->selected_shader_model)) { skip_tests = true; } @@ -1218,6 +1221,7 @@ void run_shader_tests(struct shader_runner *runner, const struct shader_runner_o state = STATE_REQUIRE; runner->minimum_shader_model = minimum_shader_model; runner->maximum_shader_model = maximum_shader_model; + runner->selected_shader_model = minimum_shader_model; runner->compile_options = 0; skip_tests = false; } diff --git a/tests/shader_runner.h b/tests/shader_runner.h index f6ef44a71..e52a44596 100644 --- a/tests/shader_runner.h +++ b/tests/shader_runner.h @@ -120,6 +120,7 @@ struct shader_runner char *cs_source; enum shader_model minimum_shader_model; enum shader_model maximum_shader_model; + enum shader_model selected_shader_model;
bool last_render_failed;
@@ -140,8 +141,9 @@ struct shader_runner
struct shader_runner_ops { - /* Returns false if unable to run the given tests. */ - bool (*check_requirements)(struct shader_runner *runner); + /* Returns false if unable to run the given tests. Otherwise, it also outputs the lowest model + * supported within the range provided in the runner. */ + bool (*check_requirements)(struct shader_runner *runner, enum shader_model *model);
struct resource *(*create_resource)(struct shader_runner *runner, const struct resource_params *params); void (*destroy_resource)(struct shader_runner *runner, struct resource *resource); diff --git a/tests/shader_runner_d3d11.c b/tests/shader_runner_d3d11.c index 7a8f8dac0..df704da8c 100644 --- a/tests/shader_runner_d3d11.c +++ b/tests/shader_runner_d3d11.c @@ -80,15 +80,13 @@ static ID3D10Blob *compile_shader(const struct d3d11_shader_runner *runner, cons
static const char *const shader_models[] = { - [SHADER_MODEL_2_0] = "4_0", - [SHADER_MODEL_3_0] = "4_0", [SHADER_MODEL_4_0] = "4_0", [SHADER_MODEL_4_1] = "4_1", [SHADER_MODEL_5_0] = "5_0", [SHADER_MODEL_5_1] = "5_1", };
- sprintf(profile, "%s_%s", type, shader_models[runner->r.minimum_shader_model]); + sprintf(profile, "%s_%s", type, shader_models[runner->r.selected_shader_model]); hr = D3DCompile(source, strlen(source), NULL, NULL, NULL, "main", profile, runner->r.compile_options, 0, &blob, &errors); ok(hr == S_OK, "Failed to compile shader, hr %#lx.\n", hr); if (errors) @@ -100,7 +98,7 @@ static ID3D10Blob *compile_shader(const struct d3d11_shader_runner *runner, cons return blob; }
-static bool d3d11_runner_check_requirements(struct shader_runner *r) +static bool d3d11_runner_check_requirements(struct shader_runner *r, enum shader_model *model) { struct d3d11_shader_runner *runner = d3d11_shader_runner(r);
@@ -109,6 +107,8 @@ static bool d3d11_runner_check_requirements(struct shader_runner *r) if (runner->r.minimum_shader_model >= SHADER_MODEL_6_0) return false;
+ *model = max(SHADER_MODEL_4_0, runner->r.minimum_shader_model); + return true; }
diff --git a/tests/shader_runner_d3d12.c b/tests/shader_runner_d3d12.c index c54cb5384..ccac95189 100644 --- a/tests/shader_runner_d3d12.c +++ b/tests/shader_runner_d3d12.c @@ -67,8 +67,6 @@ static ID3D10Blob *compile_shader(const struct d3d12_shader_runner *runner, cons
static const char *const shader_models[] = { - [SHADER_MODEL_2_0] = "4_0", - [SHADER_MODEL_3_0] = "4_0", [SHADER_MODEL_4_0] = "4_0", [SHADER_MODEL_4_1] = "4_1", [SHADER_MODEL_5_0] = "5_0", @@ -76,14 +74,14 @@ static ID3D10Blob *compile_shader(const struct d3d12_shader_runner *runner, cons [SHADER_MODEL_6_0] = "6_0", };
- if (runner->r.minimum_shader_model >= SHADER_MODEL_6_0) + if (runner->r.selected_shader_model >= SHADER_MODEL_6_0) { assert(runner->dxc_compiler); hr = dxc_compiler_compile_shader(runner->dxc_compiler, type, runner->r.compile_options, source, &blob, &errors); } else { - sprintf(profile, "%s_%s", shader_type_string(type), shader_models[runner->r.minimum_shader_model]); + sprintf(profile, "%s_%s", shader_type_string(type), shader_models[runner->r.selected_shader_model]); hr = D3DCompile(source, strlen(source), NULL, NULL, NULL, "main", profile, runner->r.compile_options, 0, &blob, &errors); } ok(FAILED(hr) == !blob, "Got unexpected hr %#x, blob %p.\n", hr, blob); @@ -96,7 +94,7 @@ static ID3D10Blob *compile_shader(const struct d3d12_shader_runner *runner, cons return blob; }
-static bool d3d12_runner_check_requirements(struct shader_runner *r) +static bool d3d12_runner_check_requirements(struct shader_runner *r, enum shader_model *model) { struct d3d12_shader_runner *runner = d3d12_shader_runner(r);
@@ -106,6 +104,8 @@ static bool d3d12_runner_check_requirements(struct shader_runner *r) if (runner->r.minimum_shader_model >= SHADER_MODEL_6_0 && !runner->dxc_compiler) return false;
+ *model = max(SHADER_MODEL_4_0, runner->r.minimum_shader_model); + return true; }
@@ -336,7 +336,7 @@ static bool d3d12_runner_dispatch(struct shader_runner *r, unsigned int x, unsig size_t i;
cs_code = compile_shader(runner, runner->r.cs_source, SHADER_TYPE_CS); - todo_if(runner->r.is_todo && runner->r.minimum_shader_model < SHADER_MODEL_6_0) ok(cs_code, "Failed to compile shader.\n"); + todo_if(runner->r.is_todo && runner->r.selected_shader_model < SHADER_MODEL_6_0) ok(cs_code, "Failed to compile shader.\n"); if (!cs_code) return false;
diff --git a/tests/shader_runner_d3d9.c b/tests/shader_runner_d3d9.c index 0b33c1c53..fbd7d6b5b 100644 --- a/tests/shader_runner_d3d9.c +++ b/tests/shader_runner_d3d9.c @@ -68,7 +68,7 @@ static ID3D10Blob *compile_shader(const struct d3d9_shader_runner *runner, const [SHADER_MODEL_3_0] = "3_0", };
- sprintf(profile, "%s_%s", type, shader_models[runner->r.minimum_shader_model]); + sprintf(profile, "%s_%s", type, shader_models[runner->r.selected_shader_model]); hr = D3DCompile(source, strlen(source), NULL, NULL, NULL, "main", profile, runner->r.compile_options, 0, &blob, &errors); ok(hr == S_OK, "Failed to compile shader, hr %#lx.\n", hr); if (errors) @@ -183,13 +183,15 @@ static D3DTEXTUREADDRESS sampler_address_to_d3d9(D3D12_TEXTURE_ADDRESS_MODE addr vkd3d_unreachable(); }
-static bool d3d9_runner_check_requirements(struct shader_runner *r) +static bool d3d9_runner_check_requirements(struct shader_runner *r, enum shader_model *model) { struct d3d9_shader_runner *runner = d3d9_shader_runner(r);
if (runner->r.minimum_shader_model >= SHADER_MODEL_4_0) return false;
+ *model = runner->r.minimum_shader_model; + return true; }
diff --git a/tests/shader_runner_vulkan.c b/tests/shader_runner_vulkan.c index ec2d1fc9e..131e394b0 100644 --- a/tests/shader_runner_vulkan.c +++ b/tests/shader_runner_vulkan.c @@ -83,7 +83,7 @@ static struct vulkan_shader_runner *vulkan_shader_runner(struct shader_runner *r
#define VK_CALL(f) (runner->f)
-static bool vulkan_runner_check_requirements(struct shader_runner *r) +static bool vulkan_runner_check_requirements(struct shader_runner *r, enum shader_model *model) { struct vulkan_shader_runner *runner = vulkan_shader_runner(r);
@@ -92,6 +92,8 @@ static bool vulkan_runner_check_requirements(struct shader_runner *r) if (runner->r.minimum_shader_model >= SHADER_MODEL_6_0) return false;
+ *model = max(SHADER_MODEL_4_0, runner->r.minimum_shader_model); + return true; }
@@ -407,8 +409,6 @@ static bool compile_shader(const struct vulkan_shader_runner *runner, const char
static const char *const shader_models[] = { - [SHADER_MODEL_2_0] = "4_0", - [SHADER_MODEL_3_0] = "4_0", [SHADER_MODEL_4_0] = "4_0", [SHADER_MODEL_4_1] = "4_1", [SHADER_MODEL_5_0] = "5_0", @@ -451,7 +451,7 @@ static bool compile_shader(const struct vulkan_shader_runner *runner, const char }
hlsl_info.entry_point = "main"; - sprintf(profile, "%s_%s", type, shader_models[runner->r.minimum_shader_model]); + sprintf(profile, "%s_%s", type, shader_models[runner->r.selected_shader_model]); hlsl_info.profile = profile;
ret = vkd3d_shader_compile(&info, dxbc, &messages);
From: "U-GPD3\fjacb" fcasas@codeweavers.com
--- tests/shader_runner.c | 14 +++++++++++--- tests/shader_runner.h | 4 ++-- tests/shader_runner_d3d11.c | 2 +- tests/shader_runner_d3d12.c | 24 ++++++++++++++++++------ tests/shader_runner_d3d9.c | 2 +- tests/shader_runner_vulkan.c | 2 +- 6 files changed, 34 insertions(+), 14 deletions(-)
diff --git a/tests/shader_runner.c b/tests/shader_runner.c index 9a4840cb0..3f4586489 100644 --- a/tests/shader_runner.c +++ b/tests/shader_runner.c @@ -955,7 +955,8 @@ static void compile_shader(struct shader_runner *runner, IDxcCompiler3 *dxc_comp
if (use_dxcompiler) { - assert(dxc_compiler); + if (!dxc_compiler) + return; hr = dxc_compiler_compile_shader(dxc_compiler, type, runner->compile_options, source, &blob, &errors); } else @@ -1030,8 +1031,8 @@ static enum parse_state read_shader_directive(struct shader_runner *runner, enum return state; }
-void run_shader_tests(struct shader_runner *runner, const struct shader_runner_ops *ops, void *dxc_compiler, - enum shader_model minimum_shader_model, enum shader_model maximum_shader_model) +static void run_shader_tests_range(struct shader_runner *runner, const struct shader_runner_ops *ops, + void *dxc_compiler, enum shader_model minimum_shader_model, enum shader_model maximum_shader_model) { size_t shader_source_size = 0, shader_source_len = 0; struct resource_params current_resource; @@ -1405,6 +1406,13 @@ void run_shader_tests(struct shader_runner *runner, const struct shader_runner_o fclose(f); }
+void run_shader_tests(struct shader_runner *runner, const struct shader_runner_ops *ops, void *dxc_compiler) +{ + run_shader_tests_range(runner, ops, dxc_compiler, SHADER_MODEL_2_0, SHADER_MODEL_3_0); + run_shader_tests_range(runner, ops, dxc_compiler, SHADER_MODEL_4_0, SHADER_MODEL_5_1); + run_shader_tests_range(runner, ops, dxc_compiler, SHADER_MODEL_6_0, SHADER_MODEL_6_0); +} + #ifdef _WIN32 static void print_dll_version(const char *file_name) { diff --git a/tests/shader_runner.h b/tests/shader_runner.h index e52a44596..fe1e24cca 100644 --- a/tests/shader_runner.h +++ b/tests/shader_runner.h @@ -165,8 +165,8 @@ void init_resource(struct resource *resource, const struct resource_params *para HRESULT dxc_compiler_compile_shader(void *dxc_compiler, enum shader_type type, unsigned int compile_options, const char *hlsl, ID3D10Blob **blob_out, ID3D10Blob **errors_out);
-void run_shader_tests(struct shader_runner *runner, const struct shader_runner_ops *ops, void *dxc_compiler, - enum shader_model minimum_shader_model, enum shader_model maximum_shader_model); +void run_shader_tests(struct shader_runner *runner, const struct shader_runner_ops *ops, + void *dxc_compiler);
#ifdef _WIN32 void run_shader_tests_d3d9(void); diff --git a/tests/shader_runner_d3d11.c b/tests/shader_runner_d3d11.c index df704da8c..4f663bc8f 100644 --- a/tests/shader_runner_d3d11.c +++ b/tests/shader_runner_d3d11.c @@ -760,7 +760,7 @@ void run_shader_tests_d3d11(void) init_adapter_info(); if (init_test_context(&runner)) { - run_shader_tests(&runner.r, &d3d11_runner_ops, NULL, SHADER_MODEL_2_0, SHADER_MODEL_5_1); + run_shader_tests(&runner.r, &d3d11_runner_ops, NULL); destroy_test_context(&runner); } } diff --git a/tests/shader_runner_d3d12.c b/tests/shader_runner_d3d12.c index ccac95189..46bdb35e9 100644 --- a/tests/shader_runner_d3d12.c +++ b/tests/shader_runner_d3d12.c @@ -51,6 +51,8 @@ struct d3d12_shader_runner ID3D12CommandAllocator *compute_allocator; ID3D12GraphicsCommandList *compute_list;
+ enum shader_model target_minimum_shader_model, target_maximum_shader_model; + IDxcCompiler3 *dxc_compiler; };
@@ -97,14 +99,21 @@ static ID3D10Blob *compile_shader(const struct d3d12_shader_runner *runner, cons static bool d3d12_runner_check_requirements(struct shader_runner *r, enum shader_model *model) { struct d3d12_shader_runner *runner = d3d12_shader_runner(r); + enum shader_model range_min, range_max; + + range_min = max(runner->target_minimum_shader_model, runner->r.minimum_shader_model); + range_max = min(runner->target_maximum_shader_model, runner->r.maximum_shader_model);
- if (runner->r.maximum_shader_model < SHADER_MODEL_4_0) + if (range_max < range_min) return false;
- if (runner->r.minimum_shader_model >= SHADER_MODEL_6_0 && !runner->dxc_compiler) + if (range_max < SHADER_MODEL_4_0) return false;
- *model = max(SHADER_MODEL_4_0, runner->r.minimum_shader_model); + if (range_min >= SHADER_MODEL_6_0 && !runner->dxc_compiler) + return false; + + *model = max(SHADER_MODEL_4_0, range_min);
return true; } @@ -589,8 +598,8 @@ static const struct shader_runner_ops d3d12_runner_ops = .release_readback = d3d12_runner_release_readback, };
-void run_shader_tests_d3d12(void *dxc_compiler, enum shader_model minimum_shader_model, - enum shader_model maximum_shader_model) +void run_shader_tests_d3d12(void *dxc_compiler, enum shader_model target_minimum_shader_model, + enum shader_model target_maximum_shader_model) { static const struct test_context_desc desc = { @@ -603,6 +612,9 @@ void run_shader_tests_d3d12(void *dxc_compiler, enum shader_model minimum_shader ID3D12Device *device; HRESULT hr;
+ runner.target_minimum_shader_model = target_minimum_shader_model; + runner.target_maximum_shader_model = target_maximum_shader_model; + enable_d3d12_debug_layer(); init_adapter_info(); if (!init_test_context(&runner.test_context, &desc)) @@ -622,7 +634,7 @@ void run_shader_tests_d3d12(void *dxc_compiler, enum shader_model minimum_shader runner.compute_allocator, NULL, &IID_ID3D12GraphicsCommandList, (void **)&runner.compute_list); ok(hr == S_OK, "Failed to create command list, hr %#x.\n", hr);
- run_shader_tests(&runner.r, &d3d12_runner_ops, dxc_compiler, minimum_shader_model, maximum_shader_model); + run_shader_tests(&runner.r, &d3d12_runner_ops, dxc_compiler);
ID3D12GraphicsCommandList_Release(runner.compute_list); ID3D12CommandAllocator_Release(runner.compute_allocator); diff --git a/tests/shader_runner_d3d9.c b/tests/shader_runner_d3d9.c index fbd7d6b5b..48b49d09a 100644 --- a/tests/shader_runner_d3d9.c +++ b/tests/shader_runner_d3d9.c @@ -543,7 +543,7 @@ void run_shader_tests_d3d9(void)
init_adapter_info(); init_test_context(&runner); - run_shader_tests(&runner.r, &d3d9_runner_ops, NULL, SHADER_MODEL_2_0, SHADER_MODEL_3_0); + run_shader_tests(&runner.r, &d3d9_runner_ops, NULL); destroy_test_context(&runner); } FreeLibrary(d3d9_module); diff --git a/tests/shader_runner_vulkan.c b/tests/shader_runner_vulkan.c index 131e394b0..79dce6bf2 100644 --- a/tests/shader_runner_vulkan.c +++ b/tests/shader_runner_vulkan.c @@ -1397,7 +1397,7 @@ void run_shader_tests_vulkan(void) if (!init_vulkan_runner(&runner)) return;
- run_shader_tests(&runner.r, &vulkan_runner_ops, NULL, SHADER_MODEL_2_0, SHADER_MODEL_5_1); + run_shader_tests(&runner.r, &vulkan_runner_ops, NULL);
cleanup_vulkan_runner(&runner); }
From: Francisco Casas fcasas@codeweavers.com
This would allow us to test compilation of SM1 shaders even if we don't have an available backend that can run them. --- tests/shader_runner.c | 69 +++++++++++++++++++++++++++++++++---------- tests/shader_runner.h | 3 +- 2 files changed, 55 insertions(+), 17 deletions(-)
diff --git a/tests/shader_runner.c b/tests/shader_runner.c index 3f4586489..6090ddcda 100644 --- a/tests/shader_runner.c +++ b/tests/shader_runner.c @@ -511,6 +511,7 @@ static void parse_test_directive(struct shader_runner *runner, const char *line) if (ret < 3) fatal_error("Malformed dispatch arguments '%s'.\n", line);
+ assert(runner->ops->dispatch); runner->last_render_failed = !runner->ops->dispatch(runner, x, y, z); } else if (match_string(line, "draw quad", &line)) @@ -543,6 +544,7 @@ static void parse_test_directive(struct shader_runner *runner, const char *line) params.height = RENDER_TARGET_HEIGHT; params.level_count = 1;
+ assert(runner->ops->create_resource); set_resource(runner, runner->ops->create_resource(runner, ¶ms)); }
@@ -562,11 +564,13 @@ static void parse_test_directive(struct shader_runner *runner, const char *line) params.data = malloc(sizeof(quad)); memcpy(params.data, quad, sizeof(quad)); params.data_size = sizeof(quad); + assert(runner->ops->create_resource); set_resource(runner, runner->ops->create_resource(runner, ¶ms));
if (!runner->vs_source) runner->vs_source = strdup(vs_source);
+ assert(runner->ops->draw); runner->last_render_failed = !runner->ops->draw(runner, D3D_PRIMITIVE_TOPOLOGY_TRIANGLELIST, 3); } else if (match_string(line, "draw", &line)) @@ -588,6 +592,7 @@ static void parse_test_directive(struct shader_runner *runner, const char *line) params.height = RENDER_TARGET_HEIGHT; params.level_count = 1;
+ assert(runner->ops->create_resource); set_resource(runner, runner->ops->create_resource(runner, ¶ms)); }
@@ -602,6 +607,7 @@ static void parse_test_directive(struct shader_runner *runner, const char *line) if (line == rest) fatal_error("Malformed vertex count '%s'.\n", line);
+ assert(runner->ops->draw); runner->last_render_failed = !runner->ops->draw(runner, topology, vertex_count); } else if (match_string(line, "probe", &line)) @@ -650,6 +656,7 @@ static void parse_test_directive(struct shader_runner *runner, const char *line) resource = get_resource(runner, RESOURCE_TYPE_RENDER_TARGET, 0); }
+ assert(runner->ops->get_resource_readback); rb = runner->ops->get_resource_readback(runner, resource);
if (match_string(line, "all", &line)) @@ -698,6 +705,7 @@ static void parse_test_directive(struct shader_runner *runner, const char *line) fatal_error("Malformed probe arguments '%s'.\n", line); }
+ assert(runner->ops->release_readback); runner->ops->release_readback(runner, rb); } else if (match_string(line, "uniform", &line)) @@ -944,8 +952,8 @@ static void compile_shader(struct shader_runner *runner, IDxcCompiler3 *dxc_comp
static const char *const shader_models[] = { - [SHADER_MODEL_2_0] = "4_0", - [SHADER_MODEL_3_0] = "4_0", + [SHADER_MODEL_2_0] = "2_0", + [SHADER_MODEL_3_0] = "3_0", [SHADER_MODEL_4_0] = "4_0", [SHADER_MODEL_4_1] = "4_1", [SHADER_MODEL_5_0] = "5_0", @@ -1032,7 +1040,8 @@ static enum parse_state read_shader_directive(struct shader_runner *runner, enum }
static void run_shader_tests_range(struct shader_runner *runner, const struct shader_runner_ops *ops, - void *dxc_compiler, enum shader_model minimum_shader_model, enum shader_model maximum_shader_model) + void *dxc_compiler, enum shader_model minimum_shader_model, enum shader_model maximum_shader_model, + bool check_syntax) { size_t shader_source_size = 0, shader_source_len = 0; struct resource_params current_resource; @@ -1057,8 +1066,11 @@ static void run_shader_tests_range(struct shader_runner *runner, const struct sh runner->maximum_shader_model = maximum_shader_model; runner->selected_shader_model = minimum_shader_model;
- assert(runner->ops->check_requirements); - skip_tests = !runner->ops->check_requirements(runner, &runner->selected_shader_model); + if (runner->ops) + { + assert(runner->ops->check_requirements); + skip_tests = !runner->ops->check_requirements(runner, &runner->selected_shader_model); + }
for (;;) { @@ -1079,7 +1091,7 @@ static void run_shader_tests_range(struct shader_runner *runner, const struct sh
case STATE_REQUIRE: if (runner->maximum_shader_model < runner->minimum_shader_model - || !runner->ops->check_requirements(runner, &runner->selected_shader_model)) + || (runner->ops && !runner->ops->check_requirements(runner, &runner->selected_shader_model))) { skip_tests = true; } @@ -1091,14 +1103,15 @@ static void run_shader_tests_range(struct shader_runner *runner, const struct sh * textures with data type other than float). */ if (!skip_tests) { - set_resource(runner, runner->ops->create_resource(runner, ¤t_resource)); + if (runner->ops) + set_resource(runner, runner->ops->create_resource(runner, ¤t_resource)); } free(current_resource.data); break;
case STATE_SHADER_COMPUTE: case STATE_SHADER_COMPUTE_TODO: - if (!skip_tests) + if (check_syntax && !skip_tests) { todo_if (state == STATE_SHADER_COMPUTE_TODO) compile_shader(runner, dxc_compiler, shader_source, shader_source_len, SHADER_TYPE_CS, @@ -1113,7 +1126,7 @@ static void run_shader_tests_range(struct shader_runner *runner, const struct sh
case STATE_SHADER_PIXEL: case STATE_SHADER_PIXEL_TODO: - if (!skip_tests) + if (check_syntax && !skip_tests) { todo_if (state == STATE_SHADER_PIXEL_TODO) compile_shader(runner, dxc_compiler, shader_source, shader_source_len, SHADER_TYPE_PS, @@ -1128,7 +1141,7 @@ static void run_shader_tests_range(struct shader_runner *runner, const struct sh
case STATE_SHADER_VERTEX: case STATE_SHADER_VERTEX_TODO: - if (!skip_tests) + if (check_syntax && !skip_tests) { todo_if (state == STATE_SHADER_VERTEX_TODO) compile_shader(runner, dxc_compiler, shader_source, shader_source_len, SHADER_TYPE_VS, @@ -1386,7 +1399,10 @@ static void run_shader_tests_range(struct shader_runner *runner, const struct sh /* Compilation which fails with dxcompiler is not 'todo', therefore the tests are * not 'todo' either. They cannot run, so skip them entirely. */ if (!skip_tests && SUCCEEDED(expect_hr)) - parse_test_directive(runner, line); + { + if (!check_syntax) + parse_test_directive(runner, line); + } break; } } @@ -1399,6 +1415,7 @@ static void run_shader_tests_range(struct shader_runner *runner, const struct sh free(runner->ps_source); for (i = 0; i < runner->resource_count; ++i) { + assert(runner->ops); if (runner->resources[i]) runner->ops->destroy_resource(runner, runner->resources[i]); } @@ -1406,11 +1423,12 @@ static void run_shader_tests_range(struct shader_runner *runner, const struct sh fclose(f); }
-void run_shader_tests(struct shader_runner *runner, const struct shader_runner_ops *ops, void *dxc_compiler) +void run_shader_tests(struct shader_runner *runner, const struct shader_runner_ops *ops, + void *dxc_compiler) { - run_shader_tests_range(runner, ops, dxc_compiler, SHADER_MODEL_2_0, SHADER_MODEL_3_0); - run_shader_tests_range(runner, ops, dxc_compiler, SHADER_MODEL_4_0, SHADER_MODEL_5_1); - run_shader_tests_range(runner, ops, dxc_compiler, SHADER_MODEL_6_0, SHADER_MODEL_6_0); + run_shader_tests_range(runner, ops, dxc_compiler, SHADER_MODEL_2_0, SHADER_MODEL_3_0, false); + run_shader_tests_range(runner, ops, dxc_compiler, SHADER_MODEL_4_0, SHADER_MODEL_5_1, false); + run_shader_tests_range(runner, ops, dxc_compiler, SHADER_MODEL_6_0, SHADER_MODEL_6_0, false); }
#ifdef _WIN32 @@ -1502,6 +1520,7 @@ static IDxcCompiler3 *dxcompiler_create(void)
START_TEST(shader_runner) { + struct shader_runner runner; #ifndef VKD3D_CROSSTEST IDxcCompiler3 *dxc_compiler; #endif @@ -1511,6 +1530,10 @@ START_TEST(shader_runner) #if defined(VKD3D_CROSSTEST) trace("Running tests from a Windows cross build\n");
+ trace("Compiling shaders with d3dcompiler_47.dll\n"); + runner = (struct shader_runner){}; + run_shader_tests_range(&runner, NULL, NULL, SHADER_MODEL_4_0, SHADER_MODEL_5_1, true); + trace("Compiling shaders with d3dcompiler_47.dll and executing with d3d9.dll\n"); run_shader_tests_d3d9();
@@ -1528,6 +1551,10 @@ START_TEST(shader_runner) #elif defined(_WIN32) trace("Running tests from a Windows non-cross build\n");
+ trace("Compiling shaders with vkd3d-shader.\n"); + runner = (struct shader_runner){}; + run_shader_tests_range(&runner, NULL, NULL, SHADER_MODEL_4_0, SHADER_MODEL_5_1, true); + trace("Compiling shaders with vkd3d-shader and executing with d3d9.dll\n"); run_shader_tests_d3d9();
@@ -1539,6 +1566,10 @@ START_TEST(shader_runner)
if ((dxc_compiler = dxcompiler_create())) { + trace("Compiling shaders with dxcompiler\n"); + runner = (struct shader_runner){}; + run_shader_tests_range(&runner, NULL, dxc_compiler, SHADER_MODEL_6_0, SHADER_MODEL_6_0, true); + trace("Compiling shaders with dxcompiler and executing with vkd3d\n"); run_shader_tests_d3d12(dxc_compiler, SHADER_MODEL_6_0, SHADER_MODEL_6_0); IDxcCompiler3_Release(dxc_compiler); @@ -1550,6 +1581,10 @@ START_TEST(shader_runner) #else trace("Running tests from a Unix build\n");
+ trace("Compiling shaders with vkd3d-shader\n"); + runner = (struct shader_runner){}; + run_shader_tests_range(&runner, NULL, NULL, SHADER_MODEL_4_0, SHADER_MODEL_5_1, true); + trace("Compiling shaders with vkd3d-shader and executing with Vulkan\n"); run_shader_tests_vulkan();
@@ -1558,6 +1593,10 @@ START_TEST(shader_runner)
if ((dxc_compiler = dxcompiler_create())) { + trace("Compiling shaders with dxcompiler\n"); + runner = (struct shader_runner){}; + run_shader_tests_range(&runner, NULL, dxc_compiler, SHADER_MODEL_6_0, SHADER_MODEL_6_0, true); + trace("Compiling shaders with dxcompiler and executing with vkd3d\n"); run_shader_tests_d3d12(dxc_compiler, SHADER_MODEL_6_0, SHADER_MODEL_6_0); IDxcCompiler3_Release(dxc_compiler); diff --git a/tests/shader_runner.h b/tests/shader_runner.h index fe1e24cca..1314733ba 100644 --- a/tests/shader_runner.h +++ b/tests/shader_runner.h @@ -165,8 +165,7 @@ void init_resource(struct resource *resource, const struct resource_params *para HRESULT dxc_compiler_compile_shader(void *dxc_compiler, enum shader_type type, unsigned int compile_options, const char *hlsl, ID3D10Blob **blob_out, ID3D10Blob **errors_out);
-void run_shader_tests(struct shader_runner *runner, const struct shader_runner_ops *ops, - void *dxc_compiler); +void run_shader_tests(struct shader_runner *runner, const struct shader_runner_ops *ops, void *dxc_compiler);
#ifdef _WIN32 void run_shader_tests_d3d9(void);
I pushed my interpretation of @zfigura's idea, but I think they conflict with @giomasce proposal of specifying every shader model individually.
In person we have agreed upon going for gio's idea for the [require] directives, specifically the one where every shader model is manually specified.
We will make the default be: ``` [require] shader model 2.0 shader model 4.0 shader model 6.0 ```
I will work on this update for the MR so I will mark the current version as a Draft.
Regarding only having one [require] directive at the beginning of the file (when we want the tests to run on shader models other than the default ones), I don't think we should make it mandatory, just encourage it.
I'm probably missing a decent chunk of the discussion at this point, but where are we going with this, and why did we want that again?
On Tue Oct 31 01:56:29 2023 +0000, Henri Verbeet wrote:
I'm probably missing a decent chunk of the discussion at this point, but where are we going with this, and why did we want that again?
I am not good with big pictures, but I think our main objective is to make the shader_runner expressive enough to support SM1 and SM6 profile tests.
The spontaneous order in which seems we will achieve this is: 1. @cmccarthy patches to test compilation (not execution yet) of tests with SM6 profiles, using DXC. Which are already upstreamed. 2. My patches to test compilation of SM1 tests (not execution yet) with SM1 profiles, using vkd3d. This series belongs to these. I am trying to get to this point to upload proper tests for the the patches to merge separate samplers in SM1. 3. @zfigura patches to actually run SM1 tests using vkd3d.
Much discussion has been towards how to achieve this expressive capability without making the shader_runner or the tests too cumbersome. This is a difficult problem because we not only have to specify which shader models each test must be run with, but also for which ones we are in `todo` state, or on which one we expect `fail` or `notimpl` (we are calling these "qualifiers"). We have to intersect these [require] "shader ranges" with the "shader ranges" that each runner is capable of running.
I have been updating this series as the concept of "shader ranges" for the shader_runner has been evolving, but after more discussion we have agreed with @gio and @zfigura to follow @giomasce's [proposal](https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/418#note_49736) of address the shader models individually, on which I am working.
I also have patches queued to improve the expressivity of the __qualifiers__.
I'm probably missing a decent chunk of the discussion at this point, but where are we going with this, and why did we want that again?
The main thing we want to do is run more test permutations. Currently we run every backend once, with the lowest shader model that is both supported by the backend and valid for the test, and then we also run with d3d12. We'd like to do the following things:
* By default, compile-test each HLSL shader with sm1/d3dbc, sm4/tpf, and sm6/dxil where possible.
* By default, run Vulkan tests with both sm4/tpf and sm6/dxil where possible. In terms of implementation, we are *almost* at the point where we can add sm1 to this list as well.
* By default, run d3d12 tests with both sm4/tpf and sm6/dxil where possible. We do this already but I'd like it to stop being a special case.
* Make it possible to run certain tests with more shader models, e.g. every model in range. E.g. semantics work slightly differently for 1.x, 2.x, and 3.x, so we want to run semantic-related tests for all of those versions, but we don't really need to run *every* test with every shader model.
Some of this was discussed in person, but that was mostly design. Of course that doesn't mean that's the design we'll settle on.
The main thing we want to do is run more test permutations. Currently we run every backend once, with the lowest shader model that is both supported by the backend and valid for the test, and then we also run with d3d12. We'd like to do the following things:
- By default, compile-test each HLSL shader with sm1/d3dbc, sm4/tpf, and sm6/dxil where possible.
- By default, run Vulkan tests with both sm4/tpf and sm6/dxil where possible. In terms of implementation, we are *almost* at the point where we can add sm1 to this list as well.
- By default, run d3d12 tests with both sm4/tpf and sm6/dxil where possible. We do this already but I'd like it to stop being a special case.
In terms of core shader runner infrastructure, I don't think there's much preventing any of these? I'd prefer run_shader_tests_d3d12() taking care of which configurations to run internally instead of the main shader runner making that decision for us, but that's somewhat of an implementation detail of individual runners. I.e., I'd imagine something like the following for the Vulkan runner:
```c void run_shader_tests_vulkan(void) { struct vulkan_runner runner;
if (!vulkan_runner_init(&runner)) return;
vulkan_runner_run(&runner, SHADER_MODEL_1_0, SHADER_MODEL_3_0); vulkan_runner_run(&runner, SHADER_MODEL_4_0, SHADER_MODEL_5_1); vulkan_runner_run(&runner, SHADER_MODEL_6_0, SHADER_MODEL_6_0);
vulkan_runner_cleanup(&runner); } ```
and you could then imagine the .shader_test files having a bunch of something like "todo(vulkan+sm1)", because the Vulkan runner can't currently run d3dbc shaders. Alternatively, we could have vulkan_runner_run() set a "I can't run these" flag for its SM1-3 run, and then have the core shader runner skip things accordingly.
I think one of the advantages of that general approach is that it can be extended to e.g. the d3d11 runner doing feature level 9_3 runs, or a hypothetical OpenGL runner doing runs using different sets of extensions, without require changes to the core infrastructure.
- Make it possible to run certain tests with more shader models, e.g. every model in range. E.g. semantics work slightly differently for 1.x, 2.x, and 3.x, so we want to run semantic-related tests for all of those versions, but we don't really need to run *every* test with every shader model.
That one is probably a little harder. I'm not sure "[require]" is the ideal place for this, but I imagine fundamentally we want something equivalent to this:
``` [loop sm=1,2,3]
...
[pixel shader] ...
[test] ...
...
[endloop] ```
or
``` [require] foreach shader model = 1.0, 2.0, 3.0
...
[require] ```
or some other variant. In any case, it doesn't seem ideal to me to impose explicitly listing individual shader models on every test if there are only a limited number of tests that need this.
Some of this was discussed in person, but that was mostly design. Of course that doesn't mean that's the design we'll settle on.
Right, but that's the part I'm mainly interested in at this point; I didn't look at the actual commits in much detail yet. (Though for what it's worth, at first sight the first two patches seem straightforward enough, while patches 3 and 4 seem perhaps less obviously desirable.)
On Tue Oct 31 15:02:48 2023 +0000, Henri Verbeet wrote:
The main thing we want to do is run more test permutations. Currently
we run every backend once, with the lowest shader model that is both supported by the backend and valid for the test, and then we also run with d3d12. We'd like to do the following things:
- By default, compile-test each HLSL shader with sm1/d3dbc, sm4/tpf,
and sm6/dxil where possible.
- By default, run Vulkan tests with both sm4/tpf and sm6/dxil where
possible. In terms of implementation, we are *almost* at the point where we can add sm1 to this list as well.
- By default, run d3d12 tests with both sm4/tpf and sm6/dxil where
possible. We do this already but I'd like it to stop being a special case. In terms of core shader runner infrastructure, I don't think there's much preventing any of these? I'd prefer run_shader_tests_d3d12() taking care of which configurations to run internally instead of the main shader runner making that decision for us, but that's somewhat of an implementation detail of individual runners. I.e., I'd imagine something like the following for the Vulkan runner:
void run_shader_tests_vulkan(void) { struct vulkan_runner runner; if (!vulkan_runner_init(&runner)) return; vulkan_runner_run(&runner, SHADER_MODEL_1_0, SHADER_MODEL_3_0); vulkan_runner_run(&runner, SHADER_MODEL_4_0, SHADER_MODEL_5_1); vulkan_runner_run(&runner, SHADER_MODEL_6_0, SHADER_MODEL_6_0); vulkan_runner_cleanup(&runner); }and you could then imagine the .shader_test files having a bunch of something like "todo(vulkan+sm1)", because the Vulkan runner can't currently run d3dbc shaders. Alternatively, we could have vulkan_runner_run() set a "I can't run these" flag for its SM1-3 run, and then have the core shader runner skip things accordingly. I think one of the advantages of that general approach is that it can be extended to e.g. the d3d11 runner doing feature level 9_3 runs, or a hypothetical OpenGL runner doing runs using different sets of extensions, without require changes to the core infrastructure.
- Make it possible to run certain tests with more shader models, e.g.
every model in range. E.g. semantics work slightly differently for 1.x, 2.x, and 3.x, so we want to run semantic-related tests for all of those versions, but we don't really need to run *every* test with every shader model. That one is probably a little harder. I'm not sure "[require]" is the ideal place for this, but I imagine fundamentally we want something equivalent to this:
[loop sm=1,2,3] ... [pixel shader] ... [test] ... ... [endloop]or
[require] foreach shader model = 1.0, 2.0, 3.0 ... [require]or some other variant. In any case, it doesn't seem ideal to me to impose explicitly listing individual shader models on every test if there are only a limited number of tests that need this.
Some of this was discussed in person, but that was mostly design. Of
course that doesn't mean that's the design we'll settle on. Right, but that's the part I'm mainly interested in at this point; I didn't look at the actual commits in much detail yet. (Though for what it's worth, at first sight the first two patches seem straightforward enough, while patches 3 and 4 seem perhaps less obviously desirable.)
FTR I just finished my interpretation of gio's proposal in !434. So I am replying referring to that implementation.
In terms of core shader runner infrastructure, I don't think there's much preventing any of these? I'd prefer run_shader_tests_d3d12() taking care of which configurations to run internally instead of the main shader runner making that decision for us [...]
The implementation in !434 iterates over all shader models, but skips when the model is not required, if it is, runner's check_requirements() is used to specify if it supports that shader model. So we would be running tests using all shader models that are enumerated in the tests, for all runners that support it.
and you could then imagine the .shader_test files having a bunch of something like "todo(vulkan+sm1)", because the Vulkan runner can't currently run d3dbc shaders. Alternatively, we could have vulkan_runner_run() set a "I can't run these" flag for its SM1-3 run, and then have the core shader runner skip things accordingly.
Patch 4/4 of this MR, which is equivalent to 2/2 of !434, separates the execution of the [test] directives with the execution of the [shader] directives, which are executed with the available compiler. So we don't qualifiers for (compiler,runner) pairs.
That one is probably a little harder. I'm not sure "[require]" is the ideal place for this, but I imagine fundamentally we want something equivalent to this: [...] or some other variant. In any case, it doesn't seem ideal to me to impose explicitly listing individual shader models on every test if there are only a limited number of tests that need this.
Well, !434 adds a little verbosity to the tests, but I don't think it is so bad. Also, to avoid writing so many [require] directives, we make the defaults as if there was a: ``` [require] shader model 2.0 shader model 4.0 shader model 6.0 ``` at the beginning of the test files.
In terms of core shader runner infrastructure, I don't think there's much preventing any of these? I'd prefer run_shader_tests_d3d12() taking care of which configurations to run internally instead of the main shader runner making that decision for us [...]
The implementation in !434 iterates over all shader models, but skips when the model is not required, if it is, runner's check_requirements() is used to specify if it supports that shader model. So we would be running tests using all shader models that are enumerated in the tests, for all runners that support it.
Right. The idea (and this was mostly my fault) is that in practice, the given constraints end up being equivalent to
``` for target in (sm1, sm4, sm6) for each backend run the tests ```
where a runner skips tests that don't make sense for it [which I think is necessary anyway], i.e. d3d9 skips anything other than sm1; d3d11 skips sm6 (and currently also sm4), etc. So this is a bit simpler than listing out every combination. It also means that we can get "run for every version" easily by just adjusting the outer loop.
That one is probably a little harder. I'm not sure "[require]" is the ideal place for this, but I imagine fundamentally we want something equivalent to this: [...] or some other variant. In any case, it doesn't seem ideal to me to impose explicitly listing individual shader models on every test if there are only a limited number of tests that need this.
Well, !434 adds a little verbosity to the tests, but I don't think it is so bad. Also, to avoid writing so many [require] directives, we make the defaults as if there was a:
[require] shader model 2.0 shader model 4.0 shader model 6.0at the beginning of the test files.
Yes, the idea is that there's an inherent default of (1, 4, 6) or something similar, so we wouldn't need to specify everywhere.
That one is probably a little harder. I'm not sure "[require]" is the ideal place for this, but I imagine fundamentally we want something equivalent to this:
[loop sm=1,2,3] ... [pixel shader] ... [test] ... ... [endloop]or
[require] foreach shader model = 1.0, 2.0, 3.0 ... [require]
I'd rather go with the second one and avoid complicating the `.shader_test` parser by introduction the concept of blocks. It might be that the name `[require]` is increasingly less sensible, but that can be solved by changing to something like `[metadata]` or `[config]`.
Another thing it might be nice to have, not necessarily now, is to gate `todo` on something else than shader model. The use case I have in mind is changing the shader runner so that under Windows it can compile shaders with vkd3d-shader and run them with native D3D implementations, or alternatively compile them with the native compiler and run them with vkd3d. In these cases it is important to know whether `todo`'s are to be attributed to the shader compiler or shader executor.
Another thing it might be nice to have, not necessarily now, is to gate `todo` on something else than shader model.
Right. I had a patch for "todo(gl)", but ended up ultimately not needing it. I'll need "todo(glsl)" soon enough though. It may be nice to have individual shader runners evaluate the condition inside todo/fail/etc., and avoid having to make changes to the core shader runner when adding/removing these; shouldn't be too hard.
This merge request was closed by Francisco Casas.
Superseded by !473.
-
Francisco Casas -
Francisco Casas (＠fcasas)
-
Giovanni Mascellani (＠giomasce)
-
Henri Verbeet (＠hverbeet)
-
U-GPD3\fjacb
-
Zebediah Figura (＠zfigura)