[PATCH 0/3] MR629: vkd3d-shader/dxil: Implement DX intrinsic GetDimensions.
From: Conor McCarthy <cmccarthy(a)codeweavers.com> Fixes regression in 1f536238a. --- libs/vkd3d-shader/dxil.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/vkd3d-shader/dxil.c b/libs/vkd3d-shader/dxil.c index ba951a6e5..291012d13 100644 --- a/libs/vkd3d-shader/dxil.c +++ b/libs/vkd3d-shader/dxil.c @@ -4725,8 +4725,8 @@ static void sm6_parser_emit_extractval(struct sm6_parser *sm6, const struct dxil vsir_instruction_init(ins, &sm6->p.location, VKD3DSIH_MOV); src_param = instruction_src_params_alloc(ins, 1, sm6); - src_param_init_scalar(src_param, elem_idx); src_param->reg = src->u.reg; + src_param_init_scalar(src_param, elem_idx); instruction_dst_param_init_ssa_scalar(ins, sm6); } -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/629
From: Conor McCarthy <cmccarthy(a)codeweavers.com> The result of resinfo in Shader Model 6 is always uint, and type casts are done later if necessary. --- libs/vkd3d-shader/d3d_asm.c | 1 + libs/vkd3d-shader/spirv.c | 38 ++++++++++++++++-------- libs/vkd3d-shader/vkd3d_shader_private.h | 1 + 3 files changed, 27 insertions(+), 13 deletions(-) diff --git a/libs/vkd3d-shader/d3d_asm.c b/libs/vkd3d-shader/d3d_asm.c index dd96b7fa5..00986a4af 100644 --- a/libs/vkd3d-shader/d3d_asm.c +++ b/libs/vkd3d-shader/d3d_asm.c @@ -1674,6 +1674,7 @@ static void shader_dump_instruction_flags(struct vkd3d_d3d_asm_compiler *compile case VKD3DSI_NONE: break; case VKD3DSI_RESINFO_RCP_FLOAT: shader_addline(buffer, "_rcpFloat"); break; case VKD3DSI_RESINFO_UINT: shader_addline(buffer, "_uint"); break; + case VKD3DSI_RESINFO_UINT_DST: shader_addline(buffer, "_uint_dst"); break; default: shader_addline(buffer, "_unrecognized(%#x)", ins->flags); } break; diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c index d4333e6d0..8ce86652c 100644 --- a/libs/vkd3d-shader/spirv.c +++ b/libs/vkd3d-shader/spirv.c @@ -9099,8 +9099,17 @@ static void spirv_compiler_emit_resinfo(struct spirv_compiler *compiler, else { val_id = vkd3d_spirv_build_op_image_query_size(builder, type_id, image.image_id); - /* For UAVs the returned miplevel count is always 1. */ - miplevel_count_id = spirv_compiler_get_constant_uint(compiler, 1); + if (image.resource_type_info->ms && instruction->flags == VKD3DSI_RESINFO_UINT_DST) + { + /* In this case resinfo must also return the sample count. */ + type_id = vkd3d_spirv_get_type_id(builder, VKD3D_SHADER_COMPONENT_UINT, 1); + miplevel_count_id = vkd3d_spirv_build_op_image_query_samples(builder, type_id, image.image_id); + } + else + { + /* For UAVs the returned miplevel count is always 1. */ + miplevel_count_id = spirv_compiler_get_constant_uint(compiler, 1); + } } constituents[0] = val_id; @@ -9111,19 +9120,22 @@ static void spirv_compiler_emit_resinfo(struct spirv_compiler *compiler, val_id = vkd3d_spirv_build_op_composite_construct(builder, type_id, constituents, i + 2); - type_id = vkd3d_spirv_get_type_id(builder, VKD3D_SHADER_COMPONENT_FLOAT, VKD3D_VEC4_SIZE); - if (instruction->flags == VKD3DSI_RESINFO_UINT) - { - val_id = vkd3d_spirv_build_op_bitcast(builder, type_id, val_id); - } - else + if (instruction->flags != VKD3DSI_RESINFO_UINT_DST) { - if (instruction->flags) - FIXME("Unhandled flags %#x.\n", instruction->flags); - val_id = vkd3d_spirv_build_op_convert_utof(builder, type_id, val_id); + type_id = vkd3d_spirv_get_type_id(builder, VKD3D_SHADER_COMPONENT_FLOAT, VKD3D_VEC4_SIZE); + if (instruction->flags == VKD3DSI_RESINFO_UINT) + { + val_id = vkd3d_spirv_build_op_bitcast(builder, type_id, val_id); + } + else + { + if (instruction->flags) + FIXME("Unhandled flags %#x.\n", instruction->flags); + val_id = vkd3d_spirv_build_op_convert_utof(builder, type_id, val_id); + } + val_id = spirv_compiler_emit_swizzle(compiler, val_id, VKD3DSP_WRITEMASK_ALL, + VKD3D_SHADER_COMPONENT_FLOAT, src[1].swizzle, dst->write_mask); } - val_id = spirv_compiler_emit_swizzle(compiler, val_id, VKD3DSP_WRITEMASK_ALL, - VKD3D_SHADER_COMPONENT_FLOAT, src[1].swizzle, dst->write_mask); spirv_compiler_emit_store_dst(compiler, dst, val_id); } diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index 51320ad0e..e44927dca 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -755,6 +755,7 @@ enum vkd3d_tessellator_domain #define VKD3DSI_INDEXED_DYNAMIC 0x4 #define VKD3DSI_RESINFO_RCP_FLOAT 0x1 #define VKD3DSI_RESINFO_UINT 0x2 +#define VKD3DSI_RESINFO_UINT_DST 0x3 #define VKD3DSI_SAMPLE_INFO_UINT 0x1 #define VKD3DSI_SAMPLER_COMPARISON_MODE 0x1 #define VKD3DSI_SHIFT_UNMASKED 0x1 -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/629
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d-shader/dxil.c | 39 ++++++++++++++++++++++++++++ tests/hlsl/getdimensions.shader_test | 4 +-- 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/libs/vkd3d-shader/dxil.c b/libs/vkd3d-shader/dxil.c index 291012d13..6fd0f2697 100644 --- a/libs/vkd3d-shader/dxil.c +++ b/libs/vkd3d-shader/dxil.c @@ -365,6 +365,7 @@ enum dx_intrinsic_opcode DX_CBUFFER_LOAD_LEGACY = 59, DX_TEXTURE_LOAD = 66, DX_BUFFER_LOAD = 68, + DX_GET_DIMENSIONS = 72, DX_DERIV_COARSEX = 83, DX_DERIV_COARSEY = 84, DX_DERIV_FINEX = 85, @@ -3808,6 +3809,40 @@ static void sm6_parser_emit_dx_create_handle(struct sm6_parser *sm6, enum dx_int ins->handler_idx = VKD3DSIH_NOP; } +static void sm6_parser_emit_dx_get_dimensions(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_params; + unsigned int is_texture, component_count; + const struct sm6_value *resource; + + resource = operands[0]; + if (!sm6_value_validate_is_handle(resource, sm6)) + return; + is_texture = resource->u.handle.d->resource_type != VKD3D_SHADER_RESOURCE_BUFFER; + + instruction_init_with_resource(ins, is_texture ? VKD3DSIH_RESINFO : VKD3DSIH_BUFINFO, resource, sm6); + + if (!(src_params = instruction_src_params_alloc(ins, 1 + is_texture, sm6))) + return; + src_param_init_vector_from_reg(&src_params[is_texture], &resource->u.handle.reg); + if (is_texture) + { + /* DXIL does not have an instrinsic for sample info, and resinfo is expected to return the sample count. + * The result is always a struct of 4 x uint32. */ + ins->flags = VKD3DSI_RESINFO_UINT_DST; + src_param_init_from_value(&src_params[0], operands[1]); + component_count = VKD3D_VEC4_SIZE; + } + else + { + component_count = 1 + (resource->u.handle.d->kind == RESOURCE_KIND_STRUCTUREDBUFFER); + } + + instruction_dst_param_init_ssa_vector(ins, component_count, sm6); +} + static enum vkd3d_shader_opcode sm6_dx_map_tertiary_op(enum dx_intrinsic_opcode op) { switch (op) @@ -4120,6 +4155,7 @@ struct sm6_dx_opcode_info e -> half/float g -> half/float/double H -> handle + D -> Dimensions S -> splitdouble v -> void o -> overloaded @@ -4144,6 +4180,7 @@ static const struct sm6_dx_opcode_info sm6_dx_op_table[] = [DX_FMAX ] = {"g", "RR", sm6_parser_emit_dx_binary}, [DX_FMIN ] = {"g", "RR", sm6_parser_emit_dx_binary}, [DX_FRC ] = {"g", "R", sm6_parser_emit_dx_unary}, + [DX_GET_DIMENSIONS ] = {"D", "Hi", sm6_parser_emit_dx_get_dimensions}, [DX_IBFE ] = {"m", "iiR", sm6_parser_emit_dx_tertiary}, [DX_IMAX ] = {"m", "RR", sm6_parser_emit_dx_binary}, [DX_IMIN ] = {"m", "RR", sm6_parser_emit_dx_binary}, @@ -4210,6 +4247,8 @@ static bool sm6_parser_validate_operand_type(struct sm6_parser *sm6, const struc return sm6_type_is_floating_point(type); case 'H': return (is_return || sm6_value_is_handle(value)) && type == sm6->handle_type; + case 'D': + return sm6_type_is_struct(type) && !strcmp(type->u.struc->name, "dx.types.Dimensions"); case 'S': return sm6_type_is_struct(type) && !strcmp(type->u.struc->name, "dx.types.splitdouble"); case 'v': diff --git a/tests/hlsl/getdimensions.shader_test b/tests/hlsl/getdimensions.shader_test index 01d12f87a..4d781b320 100644 --- a/tests/hlsl/getdimensions.shader_test +++ b/tests/hlsl/getdimensions.shader_test @@ -28,7 +28,7 @@ float4 main() : sv_target } [test] -todo(sm>=6) draw quad +draw quad probe all rgba (2.0, 3.0, 2.0, 3.0) [texture 1] @@ -53,5 +53,5 @@ float4 main() : sv_target } [test] -todo(sm>=6) draw quad +draw quad probe all rgba (2.0, 2.0, 1.0, 2.0) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/629
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/spirv.c:
else { val_id = vkd3d_spirv_build_op_image_query_size(builder, type_id, image.image_id); - /* For UAVs the returned miplevel count is always 1. */ - miplevel_count_id = spirv_compiler_get_constant_uint(compiler, 1); + if (image.resource_type_info->ms && instruction->flags == VKD3DSI_RESINFO_UINT_DST) + { + /* In this case resinfo must also return the sample count. */ + type_id = vkd3d_spirv_get_type_id(builder, VKD3D_SHADER_COMPONENT_UINT, 1); + miplevel_count_id = vkd3d_spirv_build_op_image_query_samples(builder, type_id, image.image_id); + } + else + { + /* For UAVs the returned miplevel count is always 1. */ + miplevel_count_id = spirv_compiler_get_constant_uint(compiler, 1); + }
Mmmh, I think we're packing too much content into this single bit: this doesn't just control the destination type, but also putting the sample count in the last component. And `resinfo` feels to me already quite overloaded, I wouldn't add more stuff to it. Could the DXIL frontend call `resinfo` and `sampleinfo` and then stitch the results together as needed? -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/629#note_60183
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/spirv.c:
- type_id = vkd3d_spirv_get_type_id(builder, VKD3D_SHADER_COMPONENT_FLOAT, VKD3D_VEC4_SIZE); - if (instruction->flags == VKD3DSI_RESINFO_UINT) - { - val_id = vkd3d_spirv_build_op_bitcast(builder, type_id, val_id); - } - else + if (instruction->flags != VKD3DSI_RESINFO_UINT_DST) { - if (instruction->flags) - FIXME("Unhandled flags %#x.\n", instruction->flags); - val_id = vkd3d_spirv_build_op_convert_utof(builder, type_id, val_id); + type_id = vkd3d_spirv_get_type_id(builder, VKD3D_SHADER_COMPONENT_FLOAT, VKD3D_VEC4_SIZE); + if (instruction->flags == VKD3DSI_RESINFO_UINT) + { + val_id = vkd3d_spirv_build_op_bitcast(builder, type_id, val_id); + } It doesn't look great to have both `_uint` and `_uint_dst` doing essentially the same thing in slightly different ways. I wonder why we do an explicit bitcast here, instead of just setting the return type to uint and let the general store helper deal with it. Maybe so we can pretend that the type is always float and simplify the TPF parser? If so, it's not obvious to me it's the good call, and having two flags that do essentially the same thing is one fallout of that.
Incidentally, if we end up keeping `_uint_dst` I'd rather name it `_uintDst`, so the underscore acts as a flag separator. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/629#note_60184
participants (3)
-
Conor McCarthy -
Conor McCarthy (＠cmccarthy)
-
Giovanni Mascellani (＠giomasce)