[PATCH v2 0/3] MR589: vkd3d-shader/dxil: Implement DX intrinsic SplitDouble.
-- v2: vkd3d-shader/spirv: Handle split double bitcast in spirv_compiler_emit_load_ssa_reg(). tests/shader-runner: Add split double test to 64-bit cast tests. https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/589
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d-shader/dxil.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/libs/vkd3d-shader/dxil.c b/libs/vkd3d-shader/dxil.c index 15cc380f5..62bffe9a9 100644 --- a/libs/vkd3d-shader/dxil.c +++ b/libs/vkd3d-shader/dxil.c @@ -349,6 +349,7 @@ enum dx_intrinsic_opcode DX_DERIV_COARSEY = 84, DX_DERIV_FINEX = 85, DX_DERIV_FINEY = 86, + DX_SPLIT_DOUBLE = 102, DX_LEGACY_F32TOF16 = 130, DX_LEGACY_F16TOF32 = 131, }; @@ -1713,6 +1714,11 @@ static bool sm6_type_is_f16_f32(const struct sm6_type *type) return type->class == TYPE_CLASS_FLOAT && (type->u.width == 16 || type->u.width == 32); } +static bool sm6_type_is_double(const struct sm6_type *type) +{ + return type->class == TYPE_CLASS_FLOAT && type->u.width == 64; +} + static inline bool sm6_type_is_floating_point(const struct sm6_type *type) { return type->class == TYPE_CLASS_FLOAT; @@ -3702,6 +3708,20 @@ static void sm6_parser_emit_dx_buffer_load(struct sm6_parser *sm6, enum dx_intri instruction_dst_param_init_ssa_vector(ins, VKD3D_VEC4_SIZE, sm6); } +static void sm6_parser_emit_dx_split_double(struct sm6_parser *sm6, enum dx_intrinsic_opcode op, + const struct sm6_value **operands, struct function_emission_state *state) +{ + struct vkd3d_shader_instruction *ins = state->ins; + struct vkd3d_shader_src_param *src_param; + + vsir_instruction_init(ins, &sm6->p.location, VKD3DSIH_MOV); + src_param = instruction_src_params_alloc(ins, 1, sm6); + src_param_init_from_value(src_param, operands[0]); + src_param[0].reg.data_type = VKD3D_DATA_UINT; + + instruction_dst_param_init_ssa_vector(ins, 2, sm6); +} + static void sm6_parser_emit_dx_store_output(struct sm6_parser *sm6, enum dx_intrinsic_opcode op, const struct sm6_value **operands, struct function_emission_state *state) { @@ -3771,9 +3791,11 @@ struct sm6_dx_opcode_info i -> int32 m -> int16/32/64 f -> float + d -> double e -> half/float g -> half/float/double H -> handle + S -> splitdouble v -> void o -> overloaded */ @@ -3802,6 +3824,7 @@ static const struct sm6_dx_opcode_info sm6_dx_op_table[] = [DX_ROUND_PI ] = {"g0", "g", sm6_parser_emit_dx_unary}, [DX_ROUND_Z ] = {"g0", "g", sm6_parser_emit_dx_unary}, [DX_RSQRT ] = {"g0", "g", sm6_parser_emit_dx_unary}, + [DX_SPLIT_DOUBLE ] = {"S", "d", sm6_parser_emit_dx_split_double}, [DX_SQRT ] = {"g0", "g", sm6_parser_emit_dx_unary}, [DX_STORE_OUTPUT ] = {"v", "ii8o", sm6_parser_emit_dx_store_output}, }; @@ -3832,12 +3855,16 @@ static bool sm6_parser_validate_operand_type(struct sm6_parser *sm6, const struc return sm6_type_is_i16_i32_i64(type); case 'f': return sm6_type_is_float(type); + case 'd': + return sm6_type_is_double(type); case 'e': return sm6_type_is_f16_f32(type); case 'g': return sm6_type_is_floating_point(type); case 'H': return (is_return || sm6_value_is_handle(value)) && type == sm6->handle_type; + case 'S': + return sm6_type_is_struct(type) && !ascii_strcasecmp(type->u.struc->name, "dx.types.splitdouble"); case 'v': return !type; case 'o': -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/589
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- tests/hlsl/cast-64-bit.shader_test | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/hlsl/cast-64-bit.shader_test b/tests/hlsl/cast-64-bit.shader_test index aa558ec9e..a39bbfa39 100644 --- a/tests/hlsl/cast-64-bit.shader_test +++ b/tests/hlsl/cast-64-bit.shader_test @@ -57,3 +57,20 @@ uniform 0 int64_t2 -21474836481 0x100000002 uniform 4 int4 -20 8 0 0 draw quad probe all rgba (-1.0, 2.0, 1073741824.0, 536870912.0) + + +[pixel shader todo] +uniform double2 d; + +float4 main() : sv_target +{ + uint4 result; + asuint(d.x, result.x, result.y); + asuint(d.y, result.z, result.w); + return result; +} + +[test] +uniform 0 double2 -4.5 8.500003814697266 +todo draw quad +probe all rgba (0.0, 3222405120, 2147483648, 1075904512) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/589
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d-shader/spirv.c | 6 +++++- tests/hlsl/cast-64-bit.shader_test | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c index e837fc2ef..7a5ca6f08 100644 --- a/libs/vkd3d-shader/spirv.c +++ b/libs/vkd3d-shader/spirv.c @@ -3867,7 +3867,11 @@ static uint32_t spirv_compiler_emit_load_ssa_reg(struct spirv_compiler *compiler reg_component_type = vkd3d_component_type_from_data_type(ssa->data_type); if (component_type != reg_component_type) { - type_id = vkd3d_spirv_get_type_id(builder, component_type, 1); + /* SplitDouble bitcast emits two uint from a double. */ + if (reg_component_type == VKD3D_SHADER_COMPONENT_DOUBLE && component_type == VKD3D_SHADER_COMPONENT_UINT) + type_id = vkd3d_spirv_get_type_id(builder, component_type, 2); + else + type_id = vkd3d_spirv_get_type_id(builder, component_type, 1); val_id = vkd3d_spirv_build_op_bitcast(builder, type_id, val_id); } diff --git a/tests/hlsl/cast-64-bit.shader_test b/tests/hlsl/cast-64-bit.shader_test index a39bbfa39..5625d38e2 100644 --- a/tests/hlsl/cast-64-bit.shader_test +++ b/tests/hlsl/cast-64-bit.shader_test @@ -72,5 +72,5 @@ float4 main() : sv_target [test] uniform 0 double2 -4.5 8.500003814697266 -todo draw quad +draw quad probe all rgba (0.0, 3222405120, 2147483648, 1075904512) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/589
Giovanni Mascellani (@giomasce) commented about tests/hlsl/cast-64-bit.shader_test:
uniform 4 int4 -20 8 0 0 draw quad probe all rgba (-1.0, 2.0, 1073741824.0, 536870912.0) + + +[pixel shader todo] +uniform double2 d; + +float4 main() : sv_target +{ + uint4 result; + asuint(d.x, result.x, result.y); + asuint(d.y, result.z, result.w); + return result; +} I think this test should make sense for SM5, shouldn't it?
It would also be nice to return and compare results as `uint4`. Evan had written something for that in https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/416/diffs?commit_id=62..., but it hasn't been accepted yet. Maybe that patch could be cherry picked in your MR. If so, I would advise against using `%i`, see https://gitlab.winehq.org/wine/vkd3d/-/commit/9a27df3a8ca1c43a21668b44fd3c2a.... -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/589#note_58626
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/dxil.c:
instruction_dst_param_init_ssa_vector(ins, VKD3D_VEC4_SIZE, sm6); }
+static void sm6_parser_emit_dx_split_double(struct sm6_parser *sm6, enum dx_intrinsic_opcode op, + const struct sm6_value **operands, struct function_emission_state *state) +{ + struct vkd3d_shader_instruction *ins = state->ins; + struct vkd3d_shader_src_param *src_param; + + vsir_instruction_init(ins, &sm6->p.location, VKD3DSIH_MOV); + src_param = instruction_src_params_alloc(ins, 1, sm6); + src_param_init_from_value(src_param, operands[0]);
Minor nitpick, but I'd use `&src_param[0]` instead of `src_param`. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/589#note_58629
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/dxil.c:
instruction_dst_param_init_ssa_vector(ins, VKD3D_VEC4_SIZE, sm6); }
+static void sm6_parser_emit_dx_split_double(struct sm6_parser *sm6, enum dx_intrinsic_opcode op, + const struct sm6_value **operands, struct function_emission_state *state) +{ + struct vkd3d_shader_instruction *ins = state->ins; + struct vkd3d_shader_src_param *src_param; + + vsir_instruction_init(ins, &sm6->p.location, VKD3DSIH_MOV); + src_param = instruction_src_params_alloc(ins, 1, sm6); + src_param_init_from_value(src_param, operands[0]); + src_param[0].reg.data_type = VKD3D_DATA_UINT;
Hmm, I'm not completely convinced here. This amounts to saying that the cast from `double` to `uint2` is done when reading from the register, and then MOV merely forwards the `uint2`. I'd rather decide that reading the register produce a `double`, which is then casted to `uint2` along the MOV instruction. This is probably somewhat related to the discussion in https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/550 (on whether to access TEMPs and SSAs as scalar or vec4) and some discussion, mostly happened on IRC I believe, about how to represent swizzles for 64 bit types in VSIR. I don't think I have a full opinion yet, but I feel there should be some discussion. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/589#note_58630
participants (3)
-
Conor McCarthy -
Conor McCarthy (＠cmccarthy)
-
Giovanni Mascellani (＠giomasce)