[PATCH v3 0/3] MR675: Draft: tests/shader-runner: Add a raw UAV test.
-- v3: vkd3d-shader/dxil: Convert SplitDouble IMMCONST64 src params to IMMCONST. vkd3d-shader/spirv: Support 64-bit data types for 32-bit IMMCONST registers. tests/shader-runner: Add raw UAV tests. https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/675
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- Makefile.am | 1 + tests/d3d12.c | 41 -------- .../hlsl/uav-rwbyteaddressbuffer.shader_test | 59 +++++++++++ tests/shader_runner.c | 52 ++++++++-- tests/utils.h | 97 +++++++++++++++++++ 5 files changed, 199 insertions(+), 51 deletions(-) create mode 100644 tests/hlsl/uav-rwbyteaddressbuffer.shader_test diff --git a/Makefile.am b/Makefile.am index cfb225f9c..bc7616fb2 100644 --- a/Makefile.am +++ b/Makefile.am @@ -202,6 +202,7 @@ vkd3d_shader_tests = \ tests/hlsl/uav-load.shader_test \ tests/hlsl/uav-out-param.shader_test \ tests/hlsl/uav-rwbuffer.shader_test \ + tests/hlsl/uav-rwbyteaddressbuffer.shader_test \ tests/hlsl/uav-rwstructuredbuffer.shader_test \ tests/hlsl/uav-rwtexture.shader_test \ tests/hlsl/uniform-parameters.shader_test \ diff --git a/tests/d3d12.c b/tests/d3d12.c index d7933ed63..fe42d36bb 100644 --- a/tests/d3d12.c +++ b/tests/d3d12.c @@ -52,13 +52,6 @@ static bool compare_uint16(uint16_t a, uint16_t b, unsigned int max_diff) return abs(a - b) <= max_diff; } -static bool compare_uint64(uint64_t a, uint64_t b, unsigned int max_diff) -{ - uint64_t diff = a > b ? a - b : b - a; - - return diff <= max_diff; -} - static ULONG get_refcount(void *iface) { IUnknown *unk = iface; @@ -244,11 +237,6 @@ static uint16_t get_readback_uint16(struct resource_readback *rb, unsigned int x return *(uint16_t *)get_readback_data(rb, x, y, 0, sizeof(uint16_t)); } -static uint64_t get_readback_uint64(struct resource_readback *rb, unsigned int x, unsigned int y) -{ - return *(uint64_t *)get_readback_data(rb, x, y, 0, sizeof(uint64_t)); -} - #define check_sub_resource_float(a, b, c, d, e, f) check_sub_resource_float_(__LINE__, a, b, c, d, e, f) static void check_sub_resource_float_(unsigned int line, ID3D12Resource *resource, unsigned int sub_resource_idx, ID3D12CommandQueue *queue, ID3D12GraphicsCommandList *command_list, @@ -343,35 +331,6 @@ static void check_sub_resource_uint16_(unsigned int line, ID3D12Resource *resour release_resource_readback(&rb); } -#define check_readback_data_uint64(a, b, c, d) check_readback_data_uint64_(__LINE__, a, b, c, d) -static void check_readback_data_uint64_(unsigned int line, struct resource_readback *rb, - const RECT *rect, uint64_t expected, unsigned int max_diff) -{ - RECT r = {0, 0, rb->width, rb->height}; - unsigned int x = 0, y; - bool all_match = true; - uint64_t got = 0; - - if (rect) - r = *rect; - - for (y = r.top; y < r.bottom; ++y) - { - for (x = r.left; x < r.right; ++x) - { - got = get_readback_uint64(rb, x, y); - if (!compare_uint64(got, expected, max_diff)) - { - all_match = false; - break; - } - } - if (!all_match) - break; - } - ok_(line)(all_match, "Got %#"PRIx64", expected %#"PRIx64" at (%u, %u).\n", got, expected, x, y); -} - #define check_sub_resource_uint64(a, b, c, d, e, f) check_sub_resource_uint64_(__LINE__, a, b, c, d, e, f) static void check_sub_resource_uint64_(unsigned int line, ID3D12Resource *resource, unsigned int sub_resource_idx, ID3D12CommandQueue *queue, ID3D12GraphicsCommandList *command_list, diff --git a/tests/hlsl/uav-rwbyteaddressbuffer.shader_test b/tests/hlsl/uav-rwbyteaddressbuffer.shader_test new file mode 100644 index 000000000..056f94f6e --- /dev/null +++ b/tests/hlsl/uav-rwbyteaddressbuffer.shader_test @@ -0,0 +1,59 @@ +[require] +shader model >= 5.0 + +[uav 1] +format r32 float +size (buffer, 4) + +0.1 0.2 0.3 0.4 + +[pixel shader todo] +RWByteAddressBuffer u : register(u1); + +float4 main() : sv_target +{ + u.Store(0, 10); + u.Store(4, 11.1f); + return 0; +} + +[test] +todo(sm<6) draw quad +probe uav 1 (0) ri (10) +if(sm<6) probe uav 1 (1) ri (11) +if(sm>=6) probe uav 1 (1) r (11.1) + + +[pixel shader todo] +RWByteAddressBuffer u : register(u1); + +float4 main() : sv_target +{ + u.Store(0, (double)12.2); + return 0; +} + +[test] +todo draw quad +if(sm<6) probe uav 1 (0) ri (12) +if(sm>=6) probe uav 1 (0) rd (12.2) + + +% SM 6 add support for templated Store<>(). +[require] +shader model >= 6.0 + +[pixel shader] +RWByteAddressBuffer u : register(u1); + +float4 main() : sv_target +{ + u.Store<int64_t>(0, (int64_t)-12); + u.Store<double>(8, 13.3); + return 0; +} + +[test] +todo draw quad +probe uav 1 (0) ri64 (-12) +probe uav 1 (1) rd (13.3) diff --git a/tests/shader_runner.c b/tests/shader_runner.c index d0d806508..0d6bb8ec5 100644 --- a/tests/shader_runner.c +++ b/tests/shader_runner.c @@ -639,7 +639,7 @@ static void read_uint4(const char **line, struct uvec4 *v, bool is_uniform) read_uint(line, &v->w, is_uniform); } -static void read_int64(const char **line, int64_t *i) +static void read_int64(const char **line, int64_t *i, bool is_uniform) { char *rest; int64_t val; @@ -647,14 +647,14 @@ static void read_int64(const char **line, int64_t *i) errno = 0; val = strtoll(*line, &rest, 0); - if (errno != 0 || (*rest != '\0' && !isspace((unsigned char)*rest))) + if (errno != 0 || (is_uniform && *rest != '\0' && !isspace((unsigned char)*rest))) fatal_error("Malformed int64 constant '%s'.\n", *line); *i = val; - *line = rest; + *line = rest + (!is_uniform && *rest == ','); } -static void read_uint64(const char **line, uint64_t *u) +static void read_uint64(const char **line, uint64_t *u, bool is_uniform) { char *rest; uint64_t val; @@ -662,23 +662,23 @@ static void read_uint64(const char **line, uint64_t *u) errno = 0; val = strtoull(*line, &rest, 0); - if (errno != 0 || (*rest != '\0' && !isspace((unsigned char)*rest))) + if (errno != 0 || (is_uniform && *rest != '\0' && !isspace((unsigned char)*rest))) fatal_error("Malformed uint64 constant '%s'.\n", *line); *u = val; - *line = rest; + *line = rest + (!is_uniform && *rest == ','); } static void read_int64_t2(const char **line, struct i64vec2 *v) { - read_int64(line, &v->x); - read_int64(line, &v->y); + read_int64(line, &v->x, true); + read_int64(line, &v->y, true); } static void read_uint64_t2(const char **line, struct u64vec2 *v) { - read_uint64(line, &v->x); - read_uint64(line, &v->y); + read_uint64(line, &v->x, true); + read_uint64(line, &v->y, true); } static void parse_test_directive(struct shader_runner *runner, const char *line) @@ -951,6 +951,38 @@ static void parse_test_directive(struct shader_runner *runner, const char *line) line = close_parentheses(line); todo_if(runner->is_todo) check_readback_data_uint(rb, &box, expect, 0); } + else if (match_string(line, "rui64", &line) || (is_signed = match_string(line, "ri64", &line))) + { + uint64_t expect; + D3D12_BOX box; + + box.left = rect.left; + box.right = rect.right; + box.top = rect.top; + box.bottom = rect.bottom; + box.front = 0; + box.back = 1; + if (*line != '(') + fatal_error("Malformed probe arguments '%s'.\n", line); + ++line; + if (is_signed) + read_int64(&line, (int64_t *)&expect, false); + else + read_uint64(&line, &expect, false); + line = close_parentheses(line); + todo_if(runner->is_todo) check_readback_data_uint64(rb, &box, expect, 0); + } + else if (match_string(line, "rd", &line)) + { + double expect; + + ret = sscanf(line, "( %lf ) %u", &expect, &ulps); + if (ret < 1) + fatal_error("Malformed probe arguments '%s'.\n", line); + if (ret < 2) + ulps = 0; + todo_if(runner->is_todo) check_readback_data_double(rb, &rect, expect, ulps); + } else if (match_string(line, "r", &line)) { float expect; diff --git a/tests/utils.h b/tests/utils.h index cd5215fe5..03c9b5789 100644 --- a/tests/utils.h +++ b/tests/utils.h @@ -104,6 +104,13 @@ static bool compare_uint(unsigned int x, unsigned int y, unsigned int max_diff) return diff <= max_diff; } +static bool compare_uint64(uint64_t x, uint64_t y, uint64_t max_diff) +{ + uint64_t diff = x > y ? x - y : y - x; + + return diff <= max_diff; +} + static bool compare_color(DWORD c1, DWORD c2, BYTE max_diff) { return compare_uint(c1 & 0xff, c2 & 0xff, max_diff) @@ -134,6 +141,28 @@ static bool compare_float(float f, float g, unsigned int ulps) return compare_uint(x, y, ulps); } +static bool compare_double(double f, double g, unsigned int ulps) +{ + int64_t x, y; + union + { + double f; + int64_t i; + } u; + + u.f = f; + x = u.i; + u.f = g; + y = u.i; + + if (x < 0) + x = INT64_MIN - x; + if (y < 0) + y = INT64_MIN - y; + + return compare_uint64(x, y, ulps); +} + static inline bool compare_uvec4(const struct uvec4 *v1, const struct uvec4 *v2) { return v1->x == v2->x && v1->y == v2->y && v1->z == v2->z && v1->w == v2->w; @@ -167,11 +196,21 @@ static float get_readback_float(const struct resource_readback *rb, unsigned int return *(float *)get_readback_data(rb, x, y, 0, sizeof(float)); } +static double get_readback_double(const struct resource_readback *rb, unsigned int x, unsigned int y) +{ + return *(double *)get_readback_data(rb, x, y, 0, sizeof(double)); +} + static unsigned int get_readback_uint(const struct resource_readback *rb, unsigned int x, unsigned int y, unsigned int z) { return *(unsigned int*)get_readback_data(rb, x, y, z, sizeof(unsigned int)); } +static uint64_t get_readback_uint64(const struct resource_readback *rb, unsigned int x, unsigned int y) +{ + return *(uint64_t*)get_readback_data(rb, x, y, 0, sizeof(uint64_t)); +} + static const struct vec4 *get_readback_vec4(const struct resource_readback *rb, unsigned int x, unsigned int y) { return get_readback_data(rb, x, y, 0, sizeof(struct vec4)); @@ -211,6 +250,35 @@ static inline void check_readback_data_float_(unsigned int line, const struct re ok_(line)(all_match, "Got %.8e, expected %.8e at (%u, %u).\n", got, expected, x, y); } +#define check_readback_data_double(a, b, c, d) check_readback_data_double_(__LINE__, a, b, c, d) +static inline void check_readback_data_double_(unsigned int line, const struct resource_readback *rb, + const RECT *rect, double expected, unsigned int max_diff) +{ + RECT r = {0, 0, rb->width, rb->height}; + unsigned int x = 0, y; + bool all_match = true; + double got = 0; + + if (rect) + r = *rect; + + for (y = r.top; y < r.bottom; ++y) + { + for (x = r.left; x < r.right; ++x) + { + got = get_readback_double(rb, x, y); + if (!compare_double(got, expected, max_diff)) + { + all_match = false; + break; + } + } + if (!all_match) + break; + } + ok_(line)(all_match, "Got %.15le, expected %.15le at (%u, %u).\n", got, expected, x, y); +} + #define check_readback_data_uint(a, b, c, d) check_readback_data_uint_(__LINE__, a, b, c, d) static inline void check_readback_data_uint_(unsigned int line, struct resource_readback *rb, const D3D12_BOX *box, unsigned int expected, unsigned int max_diff) @@ -245,6 +313,35 @@ static inline void check_readback_data_uint_(unsigned int line, struct resource_ ok_(line)(all_match, "Got 0x%08x, expected 0x%08x at (%u, %u, %u).\n", got, expected, x, y, z); } +#define check_readback_data_uint64(a, b, c, d) check_readback_data_uint64_(__LINE__, a, b, c, d) +static inline void check_readback_data_uint64_(unsigned int line, struct resource_readback *rb, + const D3D12_BOX *box, uint64_t expected, unsigned int max_diff) +{ + D3D12_BOX b = {0, 0, 0, rb->width, rb->height, rb->depth}; + unsigned int x = 0, y = 0; + bool all_match = true; + uint64_t got = 0; + + if (box) + b = *box; + + for (y = b.top; y < b.bottom; ++y) + { + for (x = b.left; x < b.right; ++x) + { + got = get_readback_uint64(rb, x, y); + if (!compare_uint64(got, expected, max_diff)) + { + all_match = false; + break; + } + } + if (!all_match) + break; + } + ok_(line)(all_match, "Got 0x%016"PRIx64", expected 0x%016"PRIx64" at (%u, %u).\n", got, expected, x, y); +} + #define check_readback_data_vec4(a, b, c, d) check_readback_data_vec4_(__LINE__, a, b, c, d) static inline void check_readback_data_vec4_(unsigned int line, const struct resource_readback *rb, const RECT *rect, const struct vec4 *expected, unsigned int max_diff) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/675
From: Conor McCarthy <cmccarthy(a)codeweavers.com> There is no way to tell in spirv_compiler_emit_load_reg() if the write mask is 64-bit. All loads are 32-bit except for IMMCONST64 and SSA, and the latter ignores the mask, so the only issue lies in spirv_compiler_emit_load_constant64(). Handle these as IMMCONST instead. --- libs/vkd3d-shader/spirv.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c index a3baeea75..73b1a7d66 100644 --- a/libs/vkd3d-shader/spirv.c +++ b/libs/vkd3d-shader/spirv.c @@ -3740,12 +3740,21 @@ static uint32_t spirv_compiler_emit_load_constant(struct spirv_compiler *compile const struct vkd3d_shader_register *reg, uint32_t swizzle, uint32_t write_mask) { unsigned int component_count = vsir_write_mask_component_count(write_mask); + enum vkd3d_data_type data_type = reg->data_type; uint32_t values[VKD3D_VEC4_SIZE] = {0}; unsigned int i, j; assert(reg->type == VKD3DSPR_IMMCONST); - if (reg->dimension == VSIR_DIMENSION_SCALAR) + if (data_type_is_64_bit(data_type)) + { + /* SplitDouble bitcast. */ + assert(component_count <= 2); + values[0] = reg->u.immconst_u64[0]; + values[1] = reg->u.immconst_u64[0] >> 32; + data_type = VKD3D_DATA_UINT; + } + else if (reg->dimension == VSIR_DIMENSION_SCALAR) { for (i = 0; i < component_count; ++i) values[i] = *reg->u.immconst_u32; @@ -3760,7 +3769,7 @@ static uint32_t spirv_compiler_emit_load_constant(struct spirv_compiler *compile } return spirv_compiler_get_constant(compiler, - vkd3d_component_type_from_data_type(reg->data_type), component_count, values); + vkd3d_component_type_from_data_type(data_type), component_count, values); } static uint32_t spirv_compiler_emit_load_constant64(struct spirv_compiler *compiler, -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/675
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d-shader/dxil.c | 4 ++++ tests/hlsl/uav-rwbyteaddressbuffer.shader_test | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/libs/vkd3d-shader/dxil.c b/libs/vkd3d-shader/dxil.c index ac688e85e..e5d920579 100644 --- a/libs/vkd3d-shader/dxil.c +++ b/libs/vkd3d-shader/dxil.c @@ -4452,6 +4452,10 @@ static void sm6_parser_emit_dx_split_double(struct sm6_parser *sm6, enum dx_intr if (!(src_param = instruction_src_params_alloc(ins, 1, sm6))) return; src_param_init_from_value(src_param, operands[0]); + /* For an immconst register load there is no way to determine if the write mask is 32- + * or 64-bit, so make this distinction with VKD3DSPR_IMMCONST and a 64-bit data type. */ + if (src_param->reg.type == VKD3DSPR_IMMCONST64) + src_param->reg.type = VKD3DSPR_IMMCONST; instruction_dst_param_init_ssa_vector(ins, 2, sm6); } diff --git a/tests/hlsl/uav-rwbyteaddressbuffer.shader_test b/tests/hlsl/uav-rwbyteaddressbuffer.shader_test index 056f94f6e..556d0fb31 100644 --- a/tests/hlsl/uav-rwbyteaddressbuffer.shader_test +++ b/tests/hlsl/uav-rwbyteaddressbuffer.shader_test @@ -34,7 +34,7 @@ float4 main() : sv_target } [test] -todo draw quad +todo(sm<6) draw quad if(sm<6) probe uav 1 (0) ri (12) if(sm>=6) probe uav 1 (0) rd (12.2) @@ -54,6 +54,6 @@ float4 main() : sv_target } [test] -todo draw quad +draw quad probe uav 1 (0) ri64 (-12) probe uav 1 (1) rd (13.3) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/675
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/spirv.c:
unsigned int component_count = vsir_write_mask_component_count(write_mask); + enum vkd3d_data_type data_type = reg->data_type; uint32_t values[VKD3D_VEC4_SIZE] = {0}; unsigned int i, j;
assert(reg->type == VKD3DSPR_IMMCONST);
- if (reg->dimension == VSIR_DIMENSION_SCALAR) + if (data_type_is_64_bit(data_type)) + { + /* SplitDouble bitcast. */ + assert(component_count <= 2); + values[0] = reg->u.immconst_u64[0]; + values[1] = reg->u.immconst_u64[0] >> 32; + data_type = VKD3D_DATA_UINT; + }
I'm not sure this is the right solution. The instruction generated by the DXIL frontend is this: ``` mov sr1.xy <v4:uint>, d(1.3300000000000001e+01l) <s:double> ``` This instruction seems to have all the information it needs to be correctly interpreted, without having to force the source register to be IMMCONST while it clearly contains a 64 bit constant. I rather think that the MOV handler should take into account the type width mismatch and do the repacking at that level. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/675#note_62646
On Mon Feb 26 14:15:46 2024 +0000, Giovanni Mascellani wrote:
I'm not sure this is the right solution. The instruction generated by the DXIL frontend is this: ``` mov sr1.xy <v4:uint>, d(1.3300000000000001e+01l) <s:double> ``` This instruction seems to have all the information it needs to be correctly interpreted, without having to force the source register to be IMMCONST while it clearly contains a 64 bit constant. I rather think that the MOV handler should take into account the type width mismatch and do the repacking at that level. We could do that. The MOV handler calls `spirv_compiler_emit_load_src()` and doesn't look into the details of src loading, so it's a bit untidy to partially handle that outside the helper, but the only other alternative is add a parameter to the function, which is also messy because of how many other functions call it.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/675#note_62649
On Mon Feb 26 14:50:54 2024 +0000, Conor McCarthy wrote:
We could do that. The MOV handler calls `spirv_compiler_emit_load_src()` and doesn't look into the details of src loading, so it's a bit untidy to partially handle that outside the helper, but the only other alternative is add a parameter to the function, which is also messy because of how many other functions call it. I might be missing something, but I don't think the `spirv_compiler_emit_load_src()` should change. Instead, I guess `spirv_compiler_emit_mov()` should first compare the type width for its source and destination and, if they do not match, it should fix the mask passed to `spirv_compiler_emit_load_src()` and than repack appropriately the result before storing it.
I agree that the support for 64 bit types in VSIR is still a bit hacky, at some point a proper interface (independent of the various backends and frontends) should be defined and maybe some edge cases fixed or rewritten. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/675#note_62654
participants (3)
-
Conor McCarthy -
Conor McCarthy (＠cmccarthy)
-
Giovanni Mascellani (＠giomasce)