Re: [PATCH v6 0/3] MR514: tests: Use the vulkan runner to run SM1 compilation tests.

[Henri Verbeet (＠hverbeet)]

> +static bool check_qualifier_args(const char *line, const char **const rest, enum shader_model sm)
> +{
> +    struct qualifier_conditions conds;
> +    bool first = true;
> +    bool holds = false;
> +
> +    assert(*line == '(');
> +    ++line;
> +
> +    while (*line != ')')
>      {
> -        *rest = line + len;
> -        while (isspace(**rest))
> -            ++*rest;
> +        if (!first && *line == '|')
> +            ++line;
> +        first = false;
> +
> +        conds = read_qualifier_args_conjunction(line, &line);
> +
> +        if (conds.sm_min <= sm && sm <= conds.sm_max)
> +            holds = true;
>      }
> -    return true;
> +
> +    assert(*line == ')');
> +    if (rest)
> +        *rest = line + 1;
> +
> +    return holds;
>  }

I'm not too worried about it at this point, but note that in principle we don't need to compute the minimum/maximum shader model for the condition in read_qualifier_args_conjunction(). We could just pass it "sm", and then evaluate each part of the condition individually.
> @@ -627,10 +626,16 @@ void run_shader_tests_d3d12(void *dxc_compiler, enum shader_model minimum_shader
>      hr = ID3D12Device_CheckFeatureSupport(device, D3D12_FEATURE_D3D12_OPTIONS1,
>              &runner.options1, sizeof(runner.options1));
>      ok(hr == S_OK, "Failed to check feature options1 support, hr %#x.\n", hr);
> -    if (maximum_shader_model >= SHADER_MODEL_6_0)
> -        trace("Int64ShaderOps: %u.\n", runner.options1.Int64ShaderOps);
> +    trace("Int64ShaderOps: %u.\n", runner.options1.Int64ShaderOps);
>
> -    run_shader_tests(&runner.r, &d3d12_runner_ops, dxc_compiler, minimum_shader_model, maximum_shader_model);
> +    trace("Compiling SM4 shaders with %s and executing with d3d12.dll\n", HLSL_COMPILER);
> +    run_shader_tests(&runner.r, &d3d12_runner_ops, dxc_compiler, SHADER_MODEL_4_0, SHADER_MODEL_5_1);
> +
> +    if (dxc_compiler)
> +    {
> +        trace("Compiling SM6 shaders with dxcompiler and executing with vkd3d\n");
> +        run_shader_tests(&runner.r, &d3d12_runner_ops, dxc_compiler, SHADER_MODEL_6_0, SHADER_MODEL_6_0);
> +    }

Those traces aren't quite right. We could do something similar to what we do with HLSL_COMPILER to switch between "vkd3d" and "d3d12.dll" here, or we could just use "#ifdef VKD3D_CROSSTEST" to print different traces like the original code does.
> +    if (dxc_compiler)
>          IDxcCompiler3_Release(dxc_compiler);
> -        print_dll_version(SONAME_LIBDXCOMPILER);
> -    }
>
> +    if (dxc_compiler)
> +        print_dll_version(SONAME_LIBDXCOMPILER);

We might as well merge those two "if (dxc_compiler)" blocks.
> +    trace("Compiling SM1 shaders with vkd3d-shader and executing with Vulkan\n");
> +    run_shader_tests(&runner.r, &vulkan_runner_ops, NULL, SHADER_MODEL_2_0, SHADER_MODEL_3_0);

The "SM1 shaders" wording is perhaps a bit unfortunate, given that we're passing SHADER_MODEL_2_0 as the minimum above. Applies to the d3d9 runner as well.