[PATCH v3 0/4] MR645: vkd3d-shader/dxil: Implement DX intrinsics AtomicBinOp and AtomicCompareExchange.
-- v3: vkd3d-shader/dxil: Implement DX intrinsic AtomicCompareExchange. vkd3d-shader/dxil: Implement DX intrinsic AtomicBinOp. tests/shader-runner: Add tests for UAV atomic ops.
From: Conor McCarthy cmccarthy@codeweavers.com
--- tests/shader_runner.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/tests/shader_runner.c b/tests/shader_runner.c index fe5ee5972..a9a928e16 100644 --- a/tests/shader_runner.c +++ b/tests/shader_runner.c @@ -557,7 +557,7 @@ static void set_uniforms(struct shader_runner *runner, size_t offset, size_t cou memcpy(runner->uniforms + offset, uniforms, count * sizeof(*runner->uniforms)); }
-static void read_int(const char **line, int *i) +static void read_int(const char **line, int *i, bool is_uniform) { char *rest; long val; @@ -565,7 +565,7 @@ static void read_int(const char **line, int *i) errno = 0; val = strtol(*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 int constant '%s'.\n", *line);
*i = val; @@ -595,10 +595,10 @@ static void read_uint(const char **line, unsigned int *u, bool is_uniform)
static void read_int4(const char **line, struct ivec4 *v) { - read_int(line, &v->x); - read_int(line, &v->y); - read_int(line, &v->z); - read_int(line, &v->w); + read_int(line, &v->x, true); + read_int(line, &v->y, true); + read_int(line, &v->z, true); + read_int(line, &v->w, true); }
static void read_uint4(const char **line, struct uvec4 *v, bool is_uniform) @@ -774,6 +774,7 @@ static void parse_test_directive(struct shader_runner *runner, const char *line) unsigned int left, top, right, bottom, ulps, slot; struct resource_readback *rb; struct resource *resource; + bool is_signed = false; RECT rect; int len;
@@ -858,7 +859,7 @@ static void parse_test_directive(struct shader_runner *runner, const char *line) ulps = 0; todo_if(runner->is_todo) check_readback_data_vec4(rb, &rect, &v, ulps); } - else if (match_string(line, "rui", &line)) + else if (match_string(line, "rui", &line) || (is_signed = match_string(line, "ri", &line))) { unsigned int expect; D3D12_BOX box; @@ -872,7 +873,10 @@ static void parse_test_directive(struct shader_runner *runner, const char *line) if (*line != '(') fatal_error("Malformed probe arguments '%s'.\n", line); ++line; - read_uint(&line, &expect, false); + if (is_signed) + read_int(&line, (int *)&expect, false); + else + read_uint(&line, &expect, false); line = close_parentheses(line); todo_if(runner->is_todo) check_readback_data_uint(rb, &box, expect, 0); } @@ -944,7 +948,7 @@ static void parse_test_directive(struct shader_runner *runner, const char *line) { int i;
- read_int(&line, &i); + read_int(&line, &i, true); set_uniforms(runner, offset, 1, &i); } else if (match_string(line, "uint", &line))
From: Conor McCarthy cmccarthy@codeweavers.com
--- Makefile.am | 1 + tests/hlsl/uav-atomics.shader_test | 76 ++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+) create mode 100644 tests/hlsl/uav-atomics.shader_test
diff --git a/Makefile.am b/Makefile.am index d9db8eec3..5dc11e9c6 100644 --- a/Makefile.am +++ b/Makefile.am @@ -196,6 +196,7 @@ vkd3d_shader_tests = \ tests/hlsl/trigonometry.shader_test \ tests/hlsl/trunc.shader_test \ tests/hlsl/type-names.shader_test \ + tests/hlsl/uav-atomics.shader_test \ tests/hlsl/uav-load.shader_test \ tests/hlsl/uav-out-param.shader_test \ tests/hlsl/uav-rwbuffer.shader_test \ diff --git a/tests/hlsl/uav-atomics.shader_test b/tests/hlsl/uav-atomics.shader_test new file mode 100644 index 000000000..a68420d40 --- /dev/null +++ b/tests/hlsl/uav-atomics.shader_test @@ -0,0 +1,76 @@ +[require] +shader model >= 5.0 + +[buffer uav 1] +format r32 uint +size (7, 1) + +0xd 5 6 0x10 4 4 7 + +[compute shader todo] +RWBuffer<uint> u : register(u1); + +uniform uint4 v; + + [numthreads(3, 1, 1)] +void main() +{ + InterlockedAnd(u[0], v.x); + InterlockedCompareStore(u[1], v.y, v.x); + InterlockedAdd(u[2], v.x); + InterlockedOr(u[3], v.x); + InterlockedMax(u[4], v.x); + InterlockedMin(u[5], v.x); + InterlockedXor(u[6], v.x); +} + +[test] +uniform 0 uint4 3 5 0 0 +todo dispatch 1 1 1 +probe buffer uav 1 (0, 0) rui (1) +probe buffer uav 1 (1, 0) rui (3) +probe buffer uav 1 (2, 0) rui (15) +probe buffer uav 1 (3, 0) rui (0x13) +probe buffer uav 1 (4, 0) rui (4) +probe buffer uav 1 (5, 0) rui (3) +probe buffer uav 1 (6, 0) rui (4) + +uniform 0 uint4 1 2 0 0 +todo dispatch 2 1 1 +probe buffer uav 1 (0, 0) rui (1) +probe buffer uav 1 (1, 0) rui (3) +probe buffer uav 1 (2, 0) rui (21) +probe buffer uav 1 (3, 0) rui (0x13) +probe buffer uav 1 (4, 0) rui (4) +probe buffer uav 1 (5, 0) rui (1) +probe buffer uav 1 (6, 0) rui (4) + + +[buffer uav 2] +format r32 sint +size (2, 1) + +-3 1 + +[compute shader todo] +RWBuffer<int> u : register(u2); + +uniform int4 i; + + [numthreads(3, 1, 1)] +void main() +{ + InterlockedMax(u[0], i.x); + InterlockedMin(u[1], i.y); +} + +[test] +uniform 0 int4 1 -3 0 0 +todo dispatch 1 1 1 +probe buffer uav 2 (0, 0) ri (1) +probe buffer uav 2 (1, 0) ri (-3) + +uniform 0 int4 -3 1 0 0 +todo dispatch 1 1 1 +probe buffer uav 2 (0, 0) ri (1) +probe buffer uav 2 (1, 0) ri (-3)
From: Conor McCarthy cmccarthy@codeweavers.com
--- libs/vkd3d-shader/dxil.c | 119 +++++++++++++++++++++++++++-- tests/hlsl/uav-atomics.shader_test | 4 +- 2 files changed, 116 insertions(+), 7 deletions(-)
diff --git a/libs/vkd3d-shader/dxil.c b/libs/vkd3d-shader/dxil.c index a001f6f06..54003d012 100644 --- a/libs/vkd3d-shader/dxil.c +++ b/libs/vkd3d-shader/dxil.c @@ -372,6 +372,7 @@ enum dx_intrinsic_opcode DX_TEXTURE_LOAD = 66, DX_TEXTURE_STORE = 67, DX_BUFFER_LOAD = 68, + DX_ATOMIC_BINOP = 78, DX_DERIV_COARSEX = 83, DX_DERIV_COARSEY = 84, DX_DERIV_FINEX = 85, @@ -429,6 +430,20 @@ enum dxil_predicate ICMP_SLE = 41, };
+enum dxil_atomic_binop_code +{ + ATOMIC_BINOP_ADD, + ATOMIC_BINOP_AND, + ATOMIC_BINOP_OR, + ATOMIC_BINOP_XOR, + ATOMIC_BINOP_IMIN, + ATOMIC_BINOP_IMAX, + ATOMIC_BINOP_UMIN, + ATOMIC_BINOP_UMAX, + ATOMIC_BINOP_XCHG, + ATOMIC_BINOP_INVALID, +}; + struct sm6_pointer_info { const struct sm6_type *type; @@ -2266,7 +2281,7 @@ static void src_param_init_from_value(struct vkd3d_shader_src_param *param, cons static void src_param_init_vector_from_reg(struct vkd3d_shader_src_param *param, const struct vkd3d_shader_register *reg) { - param->swizzle = VKD3D_SHADER_NO_SWIZZLE; + param->swizzle = (reg->dimension == VSIR_DIMENSION_VEC4) ? VKD3D_SHADER_NO_SWIZZLE : VKD3D_SHADER_SWIZZLE(X, X, X, X); param->modifiers = VKD3DSPSM_NONE; param->reg = *reg; } @@ -3654,11 +3669,10 @@ static bool sm6_parser_emit_composite_construct(struct sm6_parser *sm6, const st }
static bool sm6_parser_emit_coordinate_construct(struct sm6_parser *sm6, const struct sm6_value **operands, - const struct sm6_value *z_operand, struct function_emission_state *state, + unsigned int max_operands, const struct sm6_value *z_operand, struct function_emission_state *state, struct vkd3d_shader_register *reg) { const struct vkd3d_shader_register *operand_regs[VKD3D_VEC4_SIZE]; - const unsigned int max_operands = 3; unsigned int component_count;
for (component_count = 0; component_count < max_operands; ++component_count) @@ -3792,6 +3806,100 @@ static void sm6_parser_emit_dx_binary(struct sm6_parser *sm6, enum dx_intrinsic_ instruction_dst_param_init_ssa_scalar(ins, sm6); }
+static enum vkd3d_shader_opcode map_dx_atomic_binop(const struct sm6_value *operand, struct sm6_parser *sm6) +{ + uint64_t code = sm6_value_get_constant_uint(operand); + + switch (code) + { + case ATOMIC_BINOP_ADD: + return VKD3DSIH_IMM_ATOMIC_IADD; + case ATOMIC_BINOP_AND: + return VKD3DSIH_IMM_ATOMIC_AND; + case ATOMIC_BINOP_IMAX: + return VKD3DSIH_IMM_ATOMIC_IMAX; + case ATOMIC_BINOP_IMIN: + return VKD3DSIH_IMM_ATOMIC_IMIN; + case ATOMIC_BINOP_OR: + return VKD3DSIH_IMM_ATOMIC_OR; + case ATOMIC_BINOP_UMAX: + return VKD3DSIH_IMM_ATOMIC_UMAX; + case ATOMIC_BINOP_UMIN: + return VKD3DSIH_IMM_ATOMIC_UMIN; + case ATOMIC_BINOP_XCHG: + return VKD3DSIH_IMM_ATOMIC_EXCH; + case ATOMIC_BINOP_XOR: + return VKD3DSIH_IMM_ATOMIC_XOR; + /* DXIL currently doesn't use SUB and NAND. */ + default: + FIXME("Unhandled atomic binop %"PRIu64".\n", code); + vkd3d_shader_parser_error(&sm6->p, VKD3D_SHADER_ERROR_DXIL_INVALID_OPERAND, + "Operation %"PRIu64" for an atomic binop instruction is unhandled.", code); + return VKD3DSIH_INVALID; + } +} + +static void sm6_parser_emit_dx_atomic_binop(struct sm6_parser *sm6, enum dx_intrinsic_opcode op, + const struct sm6_value **operands, struct function_emission_state *state) +{ + struct sm6_value *dst = sm6_parser_get_current_value(sm6); + enum vkd3d_shader_resource_type resource_type; + struct vkd3d_shader_dst_param *dst_params; + struct vkd3d_shader_src_param *src_params; + enum vkd3d_shader_opcode handler_idx; + struct vkd3d_shader_instruction *ins; + const struct sm6_value *resource; + struct vkd3d_shader_register reg; + const unsigned int coord_idx = 2; + unsigned int i, coord_count = 1; + + resource = operands[0]; + if (!sm6_value_validate_is_handle(resource, sm6)) + return; + + if ((handler_idx = map_dx_atomic_binop(operands[1], sm6)) == VKD3DSIH_INVALID) + return; + + resource_type = resource->u.handle.d->resource_type; + if (resource_type != VKD3D_SHADER_RESOURCE_BUFFER || resource->u.handle.d->kind == RESOURCE_KIND_STRUCTUREDBUFFER) + { + coord_count = 2 + (resource_type != VKD3D_SHADER_RESOURCE_BUFFER); + if (!sm6_parser_emit_coordinate_construct(sm6, &operands[coord_idx], coord_count, NULL, state, ®)) + return; + } + else + { + reg = operands[coord_idx]->u.reg; + } + + for (i = coord_idx + coord_count; i < coord_idx + 3; ++i) + { + if (!operands[i]->is_undefined) + { + WARN("Ignoring unexpected operand.\n"); + vkd3d_shader_parser_warning(&sm6->p, VKD3D_SHADER_WARNING_DXIL_IGNORING_OPERANDS, + "Ignoring an unexpected defined operand value for atomic instruction %u.", handler_idx); + break; + } + } + + ins = state->ins; + vsir_instruction_init(ins, &sm6->p.location, handler_idx); + + if (!(src_params = instruction_src_params_alloc(ins, 2, sm6))) + return; + src_param_init_vector_from_reg(&src_params[0], ®); + src_param_init_from_value(&src_params[1], operands[5]); + + dst_params = instruction_dst_params_alloc(ins, 2, sm6); + dst_param_init(&dst_params[0]); + register_init_ssa_scalar(&dst_params[0].reg, dst->type, dst, sm6); + dst_param_init(&dst_params[1]); + dst_params[1].reg = resource->u.handle.reg; + + dst->u.reg = dst_params[0].reg; +} + static void sm6_parser_emit_dx_cbuffer_load(struct sm6_parser *sm6, enum dx_intrinsic_opcode op, const struct sm6_value **operands, struct function_emission_state *state) { @@ -4150,7 +4258,7 @@ static void sm6_parser_emit_dx_texture_load(struct sm6_parser *sm6, enum dx_intr is_uav = resource->u.handle.d->type == VKD3D_SHADER_DESCRIPTOR_TYPE_UAV;
mip_level_or_sample_count = (resource_type != VKD3D_SHADER_RESOURCE_BUFFER) ? operands[1] : NULL; - if (!sm6_parser_emit_coordinate_construct(sm6, &operands[2], + if (!sm6_parser_emit_coordinate_construct(sm6, &operands[2], 3, is_multisample ? NULL : mip_level_or_sample_count, state, &coord)) { return; @@ -4187,7 +4295,7 @@ static void sm6_parser_emit_dx_texture_store(struct sm6_parser *sm6, enum dx_int if (!sm6_value_validate_is_texture_handle(resource, op, sm6)) return;
- if (!sm6_parser_emit_coordinate_construct(sm6, &operands[1], NULL, state, &coord)) + if (!sm6_parser_emit_coordinate_construct(sm6, &operands[1], 3, NULL, state, &coord)) return;
write_mask = sm6_value_get_constant_uint(operands[8]); @@ -4254,6 +4362,7 @@ static const struct sm6_dx_opcode_info sm6_dx_op_table[] = [DX_ACOS ] = {"g", "R", sm6_parser_emit_dx_unary}, [DX_ASIN ] = {"g", "R", sm6_parser_emit_dx_unary}, [DX_ATAN ] = {"g", "R", sm6_parser_emit_dx_unary}, + [DX_ATOMIC_BINOP ] = {"o", "HciiiR", sm6_parser_emit_dx_atomic_binop}, [DX_BFREV ] = {"m", "R", sm6_parser_emit_dx_unary}, [DX_BUFFER_LOAD ] = {"o", "Hii", sm6_parser_emit_dx_buffer_load}, [DX_CBUFFER_LOAD_LEGACY ] = {"o", "Hi", sm6_parser_emit_dx_cbuffer_load}, diff --git a/tests/hlsl/uav-atomics.shader_test b/tests/hlsl/uav-atomics.shader_test index a68420d40..c3c0e7066 100644 --- a/tests/hlsl/uav-atomics.shader_test +++ b/tests/hlsl/uav-atomics.shader_test @@ -66,11 +66,11 @@ void main()
[test] uniform 0 int4 1 -3 0 0 -todo dispatch 1 1 1 +todo(sm<6) dispatch 1 1 1 probe buffer uav 2 (0, 0) ri (1) probe buffer uav 2 (1, 0) ri (-3)
uniform 0 int4 -3 1 0 0 -todo dispatch 1 1 1 +todo(sm<6) dispatch 1 1 1 probe buffer uav 2 (0, 0) ri (1) probe buffer uav 2 (1, 0) ri (-3)
From: Conor McCarthy cmccarthy@codeweavers.com
--- libs/vkd3d-shader/dxil.c | 17 ++++++++++++----- tests/hlsl/uav-atomics.shader_test | 4 ++-- 2 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/libs/vkd3d-shader/dxil.c b/libs/vkd3d-shader/dxil.c index 54003d012..d69da9deb 100644 --- a/libs/vkd3d-shader/dxil.c +++ b/libs/vkd3d-shader/dxil.c @@ -373,6 +373,7 @@ enum dx_intrinsic_opcode DX_TEXTURE_STORE = 67, DX_BUFFER_LOAD = 68, DX_ATOMIC_BINOP = 78, + DX_ATOMIC_CMP_XCHG = 79, DX_DERIV_COARSEX = 83, DX_DERIV_COARSEY = 84, DX_DERIV_FINEX = 85, @@ -3844,22 +3845,25 @@ static void sm6_parser_emit_dx_atomic_binop(struct sm6_parser *sm6, enum dx_intr { struct sm6_value *dst = sm6_parser_get_current_value(sm6); enum vkd3d_shader_resource_type resource_type; + bool is_cmp_xchg = op == DX_ATOMIC_CMP_XCHG; + unsigned int i, coord_idx, coord_count = 1; struct vkd3d_shader_dst_param *dst_params; struct vkd3d_shader_src_param *src_params; enum vkd3d_shader_opcode handler_idx; struct vkd3d_shader_instruction *ins; const struct sm6_value *resource; struct vkd3d_shader_register reg; - const unsigned int coord_idx = 2; - unsigned int i, coord_count = 1;
resource = operands[0]; if (!sm6_value_validate_is_handle(resource, sm6)) return;
- if ((handler_idx = map_dx_atomic_binop(operands[1], sm6)) == VKD3DSIH_INVALID) + if (is_cmp_xchg) + handler_idx = VKD3DSIH_IMM_ATOMIC_CMP_EXCH; + else if ((handler_idx = map_dx_atomic_binop(operands[1], sm6)) == VKD3DSIH_INVALID) return;
+ coord_idx = 2 - is_cmp_xchg; resource_type = resource->u.handle.d->resource_type; if (resource_type != VKD3D_SHADER_RESOURCE_BUFFER || resource->u.handle.d->kind == RESOURCE_KIND_STRUCTUREDBUFFER) { @@ -3886,10 +3890,12 @@ static void sm6_parser_emit_dx_atomic_binop(struct sm6_parser *sm6, enum dx_intr ins = state->ins; vsir_instruction_init(ins, &sm6->p.location, handler_idx);
- if (!(src_params = instruction_src_params_alloc(ins, 2, sm6))) + if (!(src_params = instruction_src_params_alloc(ins, 2 + is_cmp_xchg, sm6))) return; src_param_init_vector_from_reg(&src_params[0], ®); - src_param_init_from_value(&src_params[1], operands[5]); + if (is_cmp_xchg) + src_param_init_from_value(&src_params[1], operands[4]); + src_param_init_from_value(&src_params[1 + is_cmp_xchg], operands[5]);
dst_params = instruction_dst_params_alloc(ins, 2, sm6); dst_param_init(&dst_params[0]); @@ -4363,6 +4369,7 @@ static const struct sm6_dx_opcode_info sm6_dx_op_table[] = [DX_ASIN ] = {"g", "R", sm6_parser_emit_dx_unary}, [DX_ATAN ] = {"g", "R", sm6_parser_emit_dx_unary}, [DX_ATOMIC_BINOP ] = {"o", "HciiiR", sm6_parser_emit_dx_atomic_binop}, + [DX_ATOMIC_CMP_XCHG ] = {"o", "HiiiRR", sm6_parser_emit_dx_atomic_binop}, [DX_BFREV ] = {"m", "R", sm6_parser_emit_dx_unary}, [DX_BUFFER_LOAD ] = {"o", "Hii", sm6_parser_emit_dx_buffer_load}, [DX_CBUFFER_LOAD_LEGACY ] = {"o", "Hi", sm6_parser_emit_dx_cbuffer_load}, diff --git a/tests/hlsl/uav-atomics.shader_test b/tests/hlsl/uav-atomics.shader_test index c3c0e7066..aed1da6ce 100644 --- a/tests/hlsl/uav-atomics.shader_test +++ b/tests/hlsl/uav-atomics.shader_test @@ -26,7 +26,7 @@ void main()
[test] uniform 0 uint4 3 5 0 0 -todo dispatch 1 1 1 +todo(sm<6) dispatch 1 1 1 probe buffer uav 1 (0, 0) rui (1) probe buffer uav 1 (1, 0) rui (3) probe buffer uav 1 (2, 0) rui (15) @@ -36,7 +36,7 @@ probe buffer uav 1 (5, 0) rui (3) probe buffer uav 1 (6, 0) rui (4)
uniform 0 uint4 1 2 0 0 -todo dispatch 2 1 1 +todo(sm<6) dispatch 2 1 1 probe buffer uav 1 (0, 0) rui (1) probe buffer uav 1 (1, 0) rui (3) probe buffer uav 1 (2, 0) rui (21)
On Wed Feb 14 10:55:07 2024 +0000, Giovanni Mascellani wrote:
I wonder why these two are fixed only here. In theory `InterlockedMax()` and `InterlockedMin()` are already working in the commit before (and on my computer the tests indeed succeed in the test before). But the CI seems to agree with you. I don't want to waste time on this, but it's strange.
Resolved now. It must have been because it wasn't added to `Makefile.am`, and I didn't specifically test it on the `AtomicBinOp` patch.
This merge request was approved by Giovanni Mascellani.
Note that this doesn't currently apply cleanly. The tests will also require some minor adjustments after Francisco's recent shader runner changes.
-
Conor McCarthy -
Conor McCarthy (＠cmccarthy)
-
Giovanni Mascellani (＠giomasce)
-
Henri Verbeet (＠hverbeet)