[PATCH 0/5] MR295: vkd3d-shader: Extend vkd3d_shader_scan_descriptor_info, part 1/2.
There are two motivations for this: * First, the structure *almost* corresponds to D3D12_SHADER_INPUT_BIND_DESC, and if more elements were included, it could be used as-is for shader reflection. There is the quirk that currently we return scan information based on the shader instructions, whereas d3dcompiler shader reflection expects to get it from the shader reflection data (i.e. the RDEF chunk), which is particularly relevant in the case that the RDEF chunk is stripped. That said, even if we have to introduce an extra scan API to account for this difference, being able to reuse the same structure seems like a benefit. In order to reuse this structure, we need to add the following elements: - Register ID (added in part 1 of this series) - Sample count (added in part 2 of this series) - Flags or resource types to distinguish between typed, raw, and structured buffers. I have not decided which representation makes the most sense; opinions are welcome. * Second, I think it makes sense to use this reflection information internally in spirv.c (and potentially other compiler backends) to declare resources in the target environment, instead of parsing DCL instructions. The idea here is that this allows backends to be more agnostic as to how resources are declared (or inferred) in the frontend, while avoiding the need to synthesize those DCL instructions in the frontend either [especially since epenthesizing instructions is more expensive than converting them to NOPs.] In order to do that, we will need vkd3d_shader_scan_descriptor_info1 to cover everything that is currently covered by DCL instructions. This needs the same elements as above (register ID and sample count), but also: - Structure stride (added in part 2 of this series) - Constant buffer used width (added in part 2 of this series) I don't currently have a proof-of-concept using these new elements. On the other hand, since it's just an extension of an existing API, I figured that seemed less critical. This does conflict trivially with 280; I'm submitting it now since 280 is accepted, but due to Alexandre's vacation may not be committed soon, and since this is new API I'd rather get comments early anyway. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/295
From: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/vkd3d_shader_main.c | 39 +++++++++------------------ 1 file changed, 13 insertions(+), 26 deletions(-) diff --git a/libs/vkd3d-shader/vkd3d_shader_main.c b/libs/vkd3d-shader/vkd3d_shader_main.c index e8259181..1c117503 100644 --- a/libs/vkd3d-shader/vkd3d_shader_main.c +++ b/libs/vkd3d-shader/vkd3d_shader_main.c @@ -703,18 +703,23 @@ static struct vkd3d_shader_cf_info *vkd3d_shader_scan_find_innermost_loop_cf_inf return NULL; } -static struct vkd3d_shader_descriptor_info *vkd3d_shader_scan_get_uav_descriptor_info( - const struct vkd3d_shader_scan_context *context, unsigned int range_id) +static void vkd3d_shader_scan_add_uav_flag(const struct vkd3d_shader_scan_context *context, + const struct vkd3d_shader_register *reg, uint32_t flag) { + unsigned int range_id = reg->idx[0].offset; unsigned int i; + if (!context->scan_descriptor_info) + return; + for (i = 0; i < context->uav_range_count; ++i) { if (context->uav_ranges[i].id == range_id) - return &context->scan_descriptor_info->descriptors[context->uav_ranges[i].descriptor_idx]; + { + context->scan_descriptor_info->descriptors[context->uav_ranges[i].descriptor_idx].flags |= flag; + break; + } } - - return NULL; } static bool vkd3d_shader_instruction_is_uav_read(const struct vkd3d_shader_instruction *instruction) @@ -730,13 +735,7 @@ static bool vkd3d_shader_instruction_is_uav_read(const struct vkd3d_shader_instr static void vkd3d_shader_scan_record_uav_read(struct vkd3d_shader_scan_context *context, const struct vkd3d_shader_register *reg) { - struct vkd3d_shader_descriptor_info *d; - - if (!context->scan_descriptor_info) - return; - - d = vkd3d_shader_scan_get_uav_descriptor_info(context, reg->idx[0].offset); - d->flags |= VKD3D_SHADER_DESCRIPTOR_INFO_FLAG_UAV_READ; + vkd3d_shader_scan_add_uav_flag(context, reg, VKD3D_SHADER_DESCRIPTOR_INFO_FLAG_UAV_READ); } static bool vkd3d_shader_instruction_is_uav_counter(const struct vkd3d_shader_instruction *instruction) @@ -749,13 +748,7 @@ static bool vkd3d_shader_instruction_is_uav_counter(const struct vkd3d_shader_in static void vkd3d_shader_scan_record_uav_counter(struct vkd3d_shader_scan_context *context, const struct vkd3d_shader_register *reg) { - struct vkd3d_shader_descriptor_info *d; - - if (!context->scan_descriptor_info) - return; - - d = vkd3d_shader_scan_get_uav_descriptor_info(context, reg->idx[0].offset); - d->flags |= VKD3D_SHADER_DESCRIPTOR_INFO_FLAG_UAV_COUNTER; + vkd3d_shader_scan_add_uav_flag(context, reg, VKD3D_SHADER_DESCRIPTOR_INFO_FLAG_UAV_COUNTER); } static bool vkd3d_shader_instruction_is_uav_atomic_op(const struct vkd3d_shader_instruction *instruction) @@ -768,13 +761,7 @@ static bool vkd3d_shader_instruction_is_uav_atomic_op(const struct vkd3d_shader_ static void vkd3d_shader_scan_record_uav_atomic_op(struct vkd3d_shader_scan_context *context, const struct vkd3d_shader_register *reg) { - struct vkd3d_shader_descriptor_info *d; - - if (!context->scan_descriptor_info) - return; - - d = vkd3d_shader_scan_get_uav_descriptor_info(context, reg->idx[0].offset); - d->flags |= VKD3D_SHADER_DESCRIPTOR_INFO_FLAG_UAV_ATOMICS; + vkd3d_shader_scan_add_uav_flag(context, reg, VKD3D_SHADER_DESCRIPTOR_INFO_FLAG_UAV_ATOMICS); } static bool vkd3d_shader_scan_add_descriptor(struct vkd3d_shader_scan_context *context, -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/295
From: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/vkd3d_shader_main.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/libs/vkd3d-shader/vkd3d_shader_main.c b/libs/vkd3d-shader/vkd3d_shader_main.c index 1c117503..f7c8c6f8 100644 --- a/libs/vkd3d-shader/vkd3d_shader_main.c +++ b/libs/vkd3d-shader/vkd3d_shader_main.c @@ -1132,11 +1132,7 @@ static int scan_with_parser(const struct vkd3d_shader_compile_info *compile_info { instruction = &parser->instructions.elements[i]; if ((ret = vkd3d_shader_scan_instruction(&context, instruction)) < 0) - { - if (scan_descriptor_info) - vkd3d_shader_free_scan_descriptor_info(scan_descriptor_info); break; - } } for (i = 0; i < ARRAY_SIZE(parser->shader_desc.flat_constant_count); ++i) @@ -1156,13 +1152,17 @@ static int scan_with_parser(const struct vkd3d_shader_compile_info *compile_info || !vkd3d_shader_signature_from_shader_signature(&signature_info->patch_constant, &parser->shader_desc.patch_constant_signature)) { - vkd3d_shader_free_scan_signature_info(signature_info); - if (scan_descriptor_info) - vkd3d_shader_free_scan_descriptor_info(scan_descriptor_info); ret = VKD3D_ERROR_OUT_OF_MEMORY; } } + if (ret < 0) + { + if (scan_descriptor_info) + vkd3d_shader_free_scan_descriptor_info(scan_descriptor_info); + if (signature_info) + vkd3d_shader_free_scan_signature_info(signature_info); + } vkd3d_shader_scan_context_cleanup(&context); return ret; } -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/295
From: Zebediah Figura <zfigura(a)codeweavers.com> --- include/vkd3d_shader.h | 102 +++++++++++++++++++++++++- libs/vkd3d-shader/vkd3d_shader_main.c | 71 +++++++++++++++--- 2 files changed, 158 insertions(+), 15 deletions(-) diff --git a/include/vkd3d_shader.h b/include/vkd3d_shader.h index d6653d18..83c39bc8 100644 --- a/include/vkd3d_shader.h +++ b/include/vkd3d_shader.h @@ -90,6 +90,11 @@ enum vkd3d_shader_structure_type * \since 1.9 */ VKD3D_SHADER_STRUCTURE_TYPE_SCAN_SIGNATURE_INFO, + /** + * The structure is a vkd3d_shader_scan_descriptor_info1 structure. + * \since 1.9 + */ + VKD3D_SHADER_STRUCTURE_TYPE_SCAN_DESCRIPTOR_INFO1, VKD3D_FORCE_32_BIT_ENUM(VKD3D_SHADER_STRUCTURE_TYPE), }; @@ -1253,7 +1258,7 @@ struct vkd3d_shader_versioned_root_signature_desc /** * The type of a shader resource, returned as part of struct - * vkd3d_shader_descriptor_info. + * vkd3d_shader_descriptor_info and struct vkd3d_shader_scan_descriptor_info1. */ enum vkd3d_shader_resource_type { @@ -1288,7 +1293,8 @@ enum vkd3d_shader_resource_type /** * The type of the data contained in a shader resource, returned as part of - * struct vkd3d_shader_descriptor_info. All formats are 32-bit. + * struct vkd3d_shader_descriptor_info and + * struct vkd3d_shader_scan_descriptor_info1. All formats are 32-bit. */ enum vkd3d_shader_resource_data_type { @@ -1318,7 +1324,7 @@ enum vkd3d_shader_resource_data_type /** * Additional flags describing a shader descriptor, returned as part of struct - * vkd3d_shader_descriptor_info. + * vkd3d_shader_descriptor_info and struct vkd3d_shader_scan_descriptor_info1. */ enum vkd3d_shader_descriptor_info_flag { @@ -1341,6 +1347,9 @@ enum vkd3d_shader_descriptor_info_flag /** * Describes a single shader descriptor; returned as part of * struct vkd3d_shader_scan_descriptor_info. + * + * struct vkd3d_shader_descriptor_info1 is an updated version of this + * structure which returns more information. */ struct vkd3d_shader_descriptor_info { @@ -1369,11 +1378,45 @@ struct vkd3d_shader_descriptor_info unsigned int count; }; +/** + * Describes a single shader descriptor; returned as part of + * struct vkd3d_shader_scan_descriptor_info1. + */ +struct vkd3d_shader_descriptor_info1 +{ + /** Type of the descriptor (for example, SRV, CBV, UAV, or sampler). */ + enum vkd3d_shader_descriptor_type type; + /** + * Register space of the resource, or 0 if the shader does not + * support multiple register spaces. + */ + unsigned int register_space; + /** Binding (register) index of the descriptor. */ + unsigned int register_index; + /** Resource type, if applicable, including its dimension. */ + enum vkd3d_shader_resource_type resource_type; + /** Data type contained in the resource (for example, float or integer). */ + enum vkd3d_shader_resource_data_type resource_data_type; + /** + * Bitwise combination of zero or more members of + * \ref vkd3d_shader_descriptor_info_flag. + */ + unsigned int flags; + /** + * Size of this descriptor array, or 1 if a single descriptor. + * For an unbounded array this value is ~0u. + */ + unsigned int count; +}; + /** * A chained structure enumerating the descriptors declared by a shader. * * This structure extends vkd3d_shader_compile_info. * + * struct vkd3d_shader_scan_descriptor_info1 is an updated version of this + * structure which returns more information. + * * When scanning a legacy Direct3D shader, vkd3d-shader enumerates each * constant register set used by the shader as a single constant buffer * descriptor, as follows: @@ -1403,6 +1446,42 @@ struct vkd3d_shader_scan_descriptor_info unsigned int descriptor_count; }; +/** + * A chained structure enumerating the descriptors declared by a shader. + * + * This structure extends vkd3d_shader_compile_info. + * + * When scanning a legacy Direct3D shader, vkd3d-shader enumerates each + * constant register set used by the shader as a single constant buffer + * descriptor, as follows: + * - The \ref vkd3d_shader_descriptor_info.type field is set to + * VKD3D_SHADER_DESCRIPTOR_TYPE_CBV. + * - The \ref vkd3d_shader_descriptor_info.register_space field is set to zero. + * - The \ref vkd3d_shader_descriptor_info.register_index field is set to a + * member of enum vkd3d_shader_d3dbc_constant_register denoting which set + * is used. + * - The \ref vkd3d_shader_descriptor_info.count field is set to one. + * + * In summary, there may be up to three such descriptors, one for each register + * set used by the shader: float, integer, and boolean. + * + * \since 1.9 + */ +struct vkd3d_shader_scan_descriptor_info1 +{ + /** + * Input; must be set to VKD3D_SHADER_STRUCTURE_TYPE_SCAN_DESCRIPTOR_INFO1. + */ + enum vkd3d_shader_structure_type type; + /** Input; optional pointer to a structure containing further parameters. */ + const void *next; + + /** Output; returns a pointer to an array of descriptors. */ + struct vkd3d_shader_descriptor_info1 *descriptors; + /** Output; size, in elements, of \ref descriptors. */ + unsigned int descriptor_count; +}; + /** * Data type of a shader varying, returned as part of struct * vkd3d_shader_signature_element. @@ -1750,6 +1829,7 @@ VKD3D_SHADER_API const enum vkd3d_shader_target_type *vkd3d_shader_get_supported * following chained structures: * - vkd3d_shader_interface_info * - vkd3d_shader_scan_descriptor_info + * - vkd3d_shader_scan_descriptor_info1 * - vkd3d_shader_scan_signature_info * - vkd3d_shader_spirv_domain_shader_target_info * - vkd3d_shader_spirv_target_info @@ -1937,6 +2017,7 @@ VKD3D_SHADER_API int vkd3d_shader_convert_root_signature(struct vkd3d_shader_ver * \n * The DXBC_TPF scanner supports the following chained structures: * - vkd3d_shader_scan_descriptor_info + * - vkd3d_shader_scan_descriptor_info1 * - vkd3d_shader_scan_signature_info * \n * Although the \a compile_info parameter is read-only, chained structures @@ -1961,8 +2042,9 @@ VKD3D_SHADER_API int vkd3d_shader_convert_root_signature(struct vkd3d_shader_ver * \return A member of \ref vkd3d_result. */ VKD3D_SHADER_API int vkd3d_shader_scan(const struct vkd3d_shader_compile_info *compile_info, char **messages); + /** - * Free members of struct vkd3d_shader_scan_descriptor_info() allocated by + * Free members of struct vkd3d_shader_scan_descriptor_info allocated by * vkd3d_shader_scan(). * * This function may free members of vkd3d_shader_scan_descriptor_info, but @@ -1973,6 +2055,18 @@ VKD3D_SHADER_API int vkd3d_shader_scan(const struct vkd3d_shader_compile_info *c VKD3D_SHADER_API void vkd3d_shader_free_scan_descriptor_info( struct vkd3d_shader_scan_descriptor_info *scan_descriptor_info); +/** + * Free members of struct vkd3d_shader_scan_descriptor_info1 allocated by + * vkd3d_shader_scan(). + * + * This function may free members of vkd3d_shader_scan_descriptor_info1, but + * does not free the structure itself. + * + * \param scan_descriptor_info Descriptor information to free. + */ +VKD3D_SHADER_API void vkd3d_shader_free_scan_descriptor_info1( + struct vkd3d_shader_scan_descriptor_info1 *scan_descriptor_info); + /** * Read the input signature of a compiled DXBC shader, returning a structural * description which can be easily parsed by C code. diff --git a/libs/vkd3d-shader/vkd3d_shader_main.c b/libs/vkd3d-shader/vkd3d_shader_main.c index f7c8c6f8..60aea24c 100644 --- a/libs/vkd3d-shader/vkd3d_shader_main.c +++ b/libs/vkd3d-shader/vkd3d_shader_main.c @@ -579,7 +579,7 @@ static bool vkd3d_shader_signature_from_shader_signature(struct vkd3d_shader_sig struct vkd3d_shader_scan_context { - struct vkd3d_shader_scan_descriptor_info *scan_descriptor_info; + struct vkd3d_shader_scan_descriptor_info1 *scan_descriptor_info; size_t descriptors_size; struct vkd3d_shader_message_context *message_context; @@ -612,7 +612,7 @@ struct vkd3d_shader_scan_context static void vkd3d_shader_scan_context_init(struct vkd3d_shader_scan_context *context, const struct vkd3d_shader_compile_info *compile_info, - struct vkd3d_shader_scan_descriptor_info *scan_descriptor_info, + struct vkd3d_shader_scan_descriptor_info1 *scan_descriptor_info, struct vkd3d_shader_message_context *message_context) { unsigned int i; @@ -769,8 +769,8 @@ static bool vkd3d_shader_scan_add_descriptor(struct vkd3d_shader_scan_context *c enum vkd3d_shader_resource_type resource_type, enum vkd3d_shader_resource_data_type resource_data_type, unsigned int flags) { - struct vkd3d_shader_scan_descriptor_info *info = context->scan_descriptor_info; - struct vkd3d_shader_descriptor_info *d; + struct vkd3d_shader_scan_descriptor_info1 *info = context->scan_descriptor_info; + struct vkd3d_shader_descriptor_info1 *d; if (!vkd3d_array_reserve((void **)&info->descriptors, &context->descriptors_size, info->descriptor_count + 1, sizeof(*info->descriptors))) @@ -1104,24 +1104,57 @@ static int vkd3d_shader_scan_instruction(struct vkd3d_shader_scan_context *conte return VKD3D_OK; } +static enum vkd3d_result convert_descriptor_info(struct vkd3d_shader_scan_descriptor_info *info, + const struct vkd3d_shader_scan_descriptor_info1 *info1) +{ + unsigned int i; + + if (!(info->descriptors = vkd3d_malloc(info1->descriptor_count * sizeof(*info->descriptors)))) + return VKD3D_ERROR_OUT_OF_MEMORY; + + for (i = 0; i < info1->descriptor_count; ++i) + { + const struct vkd3d_shader_descriptor_info1 *src = &info1->descriptors[i]; + struct vkd3d_shader_descriptor_info *dst = &info->descriptors[i]; + + dst->type = src->type; + dst->register_space = src->register_space; + dst->register_index = src->register_index; + dst->resource_type = src->resource_type; + dst->resource_data_type = src->resource_data_type; + dst->flags = src->flags; + dst->count = src->count; + } + info->descriptor_count = info1->descriptor_count; + + return VKD3D_OK; +} + static int scan_with_parser(const struct vkd3d_shader_compile_info *compile_info, struct vkd3d_shader_message_context *message_context, struct vkd3d_shader_parser *parser) { - struct vkd3d_shader_scan_descriptor_info *scan_descriptor_info; + struct vkd3d_shader_scan_descriptor_info1 *descriptor_info1, local_descriptor_info1 = {0}; + struct vkd3d_shader_scan_descriptor_info *descriptor_info; struct vkd3d_shader_scan_signature_info *signature_info; struct vkd3d_shader_instruction *instruction; struct vkd3d_shader_scan_context context; int ret = VKD3D_OK; unsigned int i; - if ((scan_descriptor_info = vkd3d_find_struct(compile_info->next, SCAN_DESCRIPTOR_INFO))) + descriptor_info = vkd3d_find_struct(compile_info->next, SCAN_DESCRIPTOR_INFO); + descriptor_info1 = vkd3d_find_struct(compile_info->next, SCAN_DESCRIPTOR_INFO1); + if (descriptor_info1) { - scan_descriptor_info->descriptors = NULL; - scan_descriptor_info->descriptor_count = 0; + descriptor_info1->descriptors = NULL; + descriptor_info1->descriptor_count = 0; + } + else if (descriptor_info) + { + descriptor_info1 = &local_descriptor_info1; } signature_info = vkd3d_find_struct(compile_info->next, SCAN_SIGNATURE_INFO); - vkd3d_shader_scan_context_init(&context, compile_info, scan_descriptor_info, message_context); + vkd3d_shader_scan_context_init(&context, compile_info, descriptor_info1, message_context); if (TRACE_ON()) { @@ -1156,13 +1189,22 @@ static int scan_with_parser(const struct vkd3d_shader_compile_info *compile_info } } + if (!ret && descriptor_info) + ret = convert_descriptor_info(descriptor_info, descriptor_info1); + if (ret < 0) { - if (scan_descriptor_info) - vkd3d_shader_free_scan_descriptor_info(scan_descriptor_info); + if (descriptor_info) + vkd3d_shader_free_scan_descriptor_info(descriptor_info); + if (descriptor_info1) + vkd3d_shader_free_scan_descriptor_info1(descriptor_info1); if (signature_info) vkd3d_shader_free_scan_signature_info(signature_info); } + else + { + vkd3d_shader_free_scan_descriptor_info1(&local_descriptor_info1); + } vkd3d_shader_scan_context_cleanup(&context); return ret; } @@ -1451,6 +1493,13 @@ void vkd3d_shader_free_scan_descriptor_info(struct vkd3d_shader_scan_descriptor_ vkd3d_free(scan_descriptor_info->descriptors); } +void vkd3d_shader_free_scan_descriptor_info1(struct vkd3d_shader_scan_descriptor_info1 *scan_descriptor_info) +{ + TRACE("scan_descriptor_info %p.\n", scan_descriptor_info); + + vkd3d_free(scan_descriptor_info->descriptors); +} + void vkd3d_shader_free_scan_signature_info(struct vkd3d_shader_scan_signature_info *info) { TRACE("info %p.\n", info); -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/295
From: Zebediah Figura <zfigura(a)codeweavers.com> --- include/vkd3d_shader.h | 6 ++++++ libs/vkd3d-shader/vkd3d_shader_main.c | 17 ++++++++++------- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/include/vkd3d_shader.h b/include/vkd3d_shader.h index 83c39bc8..e209355e 100644 --- a/include/vkd3d_shader.h +++ b/include/vkd3d_shader.h @@ -1393,6 +1393,12 @@ struct vkd3d_shader_descriptor_info1 unsigned int register_space; /** Binding (register) index of the descriptor. */ unsigned int register_index; + /** + * Register ID of the resource used internally in the shader. + * If the source format does not use distinct register IDs, this is set to + * the same value as \ref register_index. + */ + unsigned int register_id; /** Resource type, if applicable, including its dimension. */ enum vkd3d_shader_resource_type resource_type; /** Data type contained in the resource (for example, float or integer). */ diff --git a/libs/vkd3d-shader/vkd3d_shader_main.c b/libs/vkd3d-shader/vkd3d_shader_main.c index 60aea24c..0a74567a 100644 --- a/libs/vkd3d-shader/vkd3d_shader_main.c +++ b/libs/vkd3d-shader/vkd3d_shader_main.c @@ -765,9 +765,9 @@ static void vkd3d_shader_scan_record_uav_atomic_op(struct vkd3d_shader_scan_cont } static bool vkd3d_shader_scan_add_descriptor(struct vkd3d_shader_scan_context *context, - enum vkd3d_shader_descriptor_type type, const struct vkd3d_shader_register_range *range, - enum vkd3d_shader_resource_type resource_type, enum vkd3d_shader_resource_data_type resource_data_type, - unsigned int flags) + enum vkd3d_shader_descriptor_type type, const struct vkd3d_shader_register *reg, + const struct vkd3d_shader_register_range *range, enum vkd3d_shader_resource_type resource_type, + enum vkd3d_shader_resource_data_type resource_data_type, unsigned int flags) { struct vkd3d_shader_scan_descriptor_info1 *info = context->scan_descriptor_info; struct vkd3d_shader_descriptor_info1 *d; @@ -781,6 +781,7 @@ static bool vkd3d_shader_scan_add_descriptor(struct vkd3d_shader_scan_context *c d = &info->descriptors[info->descriptor_count]; d->type = type; + d->register_id = reg->idx[0].offset; d->register_space = range->space; d->register_index = range->first; d->resource_type = resource_type; @@ -817,7 +818,7 @@ static void vkd3d_shader_scan_constant_buffer_declaration(struct vkd3d_shader_sc if (!context->scan_descriptor_info) return; - vkd3d_shader_scan_add_descriptor(context, VKD3D_SHADER_DESCRIPTOR_TYPE_CBV, &cb->range, + vkd3d_shader_scan_add_descriptor(context, VKD3D_SHADER_DESCRIPTOR_TYPE_CBV, &cb->src.reg, &cb->range, VKD3D_SHADER_RESOURCE_BUFFER, VKD3D_SHADER_RESOURCE_DATA_UINT, 0); } @@ -834,7 +835,7 @@ static void vkd3d_shader_scan_sampler_declaration(struct vkd3d_shader_scan_conte flags = VKD3D_SHADER_DESCRIPTOR_INFO_FLAG_SAMPLER_COMPARISON_MODE; else flags = 0; - vkd3d_shader_scan_add_descriptor(context, VKD3D_SHADER_DESCRIPTOR_TYPE_SAMPLER, &sampler->range, + vkd3d_shader_scan_add_descriptor(context, VKD3D_SHADER_DESCRIPTOR_TYPE_SAMPLER, &sampler->src.reg, &sampler->range, VKD3D_SHADER_RESOURCE_NONE, VKD3D_SHADER_RESOURCE_DATA_UINT, flags); } @@ -851,7 +852,8 @@ static void vkd3d_shader_scan_resource_declaration(struct vkd3d_shader_scan_cont type = VKD3D_SHADER_DESCRIPTOR_TYPE_UAV; else type = VKD3D_SHADER_DESCRIPTOR_TYPE_SRV; - vkd3d_shader_scan_add_descriptor(context, type, &resource->range, resource_type, resource_data_type, 0); + vkd3d_shader_scan_add_descriptor(context, type, &resource->reg.reg, &resource->range, + resource_type, resource_data_type, 0); if (type == VKD3D_SHADER_DESCRIPTOR_TYPE_UAV) vkd3d_shader_scan_add_uav_range(context, resource->reg.reg.idx[0].offset, context->scan_descriptor_info->descriptor_count - 1); @@ -1171,9 +1173,10 @@ static int scan_with_parser(const struct vkd3d_shader_compile_info *compile_info for (i = 0; i < ARRAY_SIZE(parser->shader_desc.flat_constant_count); ++i) { struct vkd3d_shader_register_range range = {.space = 0, .first = i, .last = i}; + struct vkd3d_shader_register reg = {.idx[0].offset = i, .idx_count = 1}; if (parser->shader_desc.flat_constant_count[i].external) - vkd3d_shader_scan_add_descriptor(&context, VKD3D_SHADER_DESCRIPTOR_TYPE_CBV, + vkd3d_shader_scan_add_descriptor(&context, VKD3D_SHADER_DESCRIPTOR_TYPE_CBV, ®, &range, VKD3D_SHADER_RESOURCE_BUFFER, VKD3D_SHADER_RESOURCE_DATA_UINT, 0); } -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/295
From: Zebediah Figura <zfigura(a)codeweavers.com> This is now redundant; the register ID is encoded into the scan descriptors. --- libs/vkd3d-shader/vkd3d_shader_main.c | 35 +++------------------------ 1 file changed, 3 insertions(+), 32 deletions(-) diff --git a/libs/vkd3d-shader/vkd3d_shader_main.c b/libs/vkd3d-shader/vkd3d_shader_main.c index 0a74567a..53807a82 100644 --- a/libs/vkd3d-shader/vkd3d_shader_main.c +++ b/libs/vkd3d-shader/vkd3d_shader_main.c @@ -599,14 +599,6 @@ struct vkd3d_shader_scan_context size_t cf_info_size; size_t cf_info_count; - struct - { - unsigned int id; - unsigned int descriptor_idx; - } *uav_ranges; - size_t uav_ranges_size; - size_t uav_range_count; - enum vkd3d_shader_api_version api_version; }; @@ -635,7 +627,6 @@ static void vkd3d_shader_scan_context_init(struct vkd3d_shader_scan_context *con static void vkd3d_shader_scan_context_cleanup(struct vkd3d_shader_scan_context *context) { - vkd3d_free(context->uav_ranges); vkd3d_free(context->cf_info); } @@ -712,11 +703,11 @@ static void vkd3d_shader_scan_add_uav_flag(const struct vkd3d_shader_scan_contex if (!context->scan_descriptor_info) return; - for (i = 0; i < context->uav_range_count; ++i) + for (i = 0; i < context->scan_descriptor_info->descriptor_count; ++i) { - if (context->uav_ranges[i].id == range_id) + if (context->scan_descriptor_info->descriptors[i].register_id == range_id) { - context->scan_descriptor_info->descriptors[context->uav_ranges[i].descriptor_idx].flags |= flag; + context->scan_descriptor_info->descriptors[i].flags |= flag; break; } } @@ -793,23 +784,6 @@ static bool vkd3d_shader_scan_add_descriptor(struct vkd3d_shader_scan_context *c return true; } -static bool vkd3d_shader_scan_add_uav_range(struct vkd3d_shader_scan_context *context, - unsigned int id, unsigned int descriptor_idx) -{ - if (!vkd3d_array_reserve((void **)&context->uav_ranges, &context->uav_ranges_size, - context->uav_range_count + 1, sizeof(*context->uav_ranges))) - { - ERR("Failed to allocate UAV range.\n"); - return false; - } - - context->uav_ranges[context->uav_range_count].id = id; - context->uav_ranges[context->uav_range_count].descriptor_idx = descriptor_idx; - ++context->uav_range_count; - - return true; -} - static void vkd3d_shader_scan_constant_buffer_declaration(struct vkd3d_shader_scan_context *context, const struct vkd3d_shader_instruction *instruction) { @@ -854,9 +828,6 @@ static void vkd3d_shader_scan_resource_declaration(struct vkd3d_shader_scan_cont type = VKD3D_SHADER_DESCRIPTOR_TYPE_SRV; vkd3d_shader_scan_add_descriptor(context, type, &resource->reg.reg, &resource->range, resource_type, resource_data_type, 0); - if (type == VKD3D_SHADER_DESCRIPTOR_TYPE_UAV) - vkd3d_shader_scan_add_uav_range(context, resource->reg.reg.idx[0].offset, - context->scan_descriptor_info->descriptor_count - 1); } static void vkd3d_shader_scan_typed_resource_declaration(struct vkd3d_shader_scan_context *context, -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/295
Broadly, this probably makes sense; there may be an open question whether this should be called vkd3d_shader_scan_descriptor_info1 or something else, and whether we should duplicate the original structure or just provide the extra information separately, but I don't currently have a strong opinion on that. I do wonder about the reason for submitting this right now; my impression is that the shader reflection bits aren't in danger of being imminently released, so I imagine this is mostly intended to address the second point. That's a purely internal usage though, and wouldn't require introducing public API. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/295#note_41164
Broadly, this probably makes sense; there may be an open question whether this should be called vkd3d_shader_scan_descriptor_info1 or something else, and whether we should duplicate the original structure or just provide the extra information separately, but I don't currently have a strong opinion on that.
I considered not duplicating the original information, but it struck me as awkward, or at least, unidiomatic. It would require a caller to provide both structures instead of just one, and it would also mean they'd have to index two separate arrays. As for extending with a -1, well, we do have precedent for that.
I do wonder about the reason for submitting this right now; my impression is that the shader reflection bits aren't in danger of being imminently released, so I imagine this is mostly intended to address the second point. That's a purely internal usage though, and wouldn't require introducing public API.
Sure, and I suppose I could just make the structure private for now. I didn't see a reason to do that, though. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/295#note_41222
Broadly, this probably makes sense; there may be an open question whether this should be called vkd3d_shader_scan_descriptor_info1 or something else, and whether we should duplicate the original structure or just provide the extra information separately, but I don't currently have a strong opinion on that.
I considered not duplicating the original information, but it struck me as awkward, or at least, unidiomatic. It would require a caller to provide both structures instead of just one, and it would also mean they'd have to index two separate arrays.
As for extending with a -1, well, we do have precedent for that.
Sure; I guess I'm mostly just saying I haven't given the question much thought, and at some point I probably should.
I do wonder about the reason for submitting this right now; my impression is that the shader reflection bits aren't in danger of being imminently released, so I imagine this is mostly intended to address the second point. That's a purely internal usage though, and wouldn't require introducing public API.
Sure, and I suppose I could just make the structure private for now. I didn't see a reason to do that, though.
Mostly just private structures being easier than public ones, as well as getting the chance to see how using this internally for SPIR-V ends up working out. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/295#note_41254
I do wonder about the reason for submitting this right now; my impression is that the shader reflection bits aren't in danger of being imminently released, so I imagine this is mostly intended to address the second point. That's a purely internal usage though, and wouldn't require introducing public API.
Sure, and I suppose I could just make the structure private for now. I didn't see a reason to do that, though.
Mostly just private structures being easier than public ones, as well as getting the chance to see how using this internally for SPIR-V ends up working out.
I can see the value in avoiding exposing the API unless necessary. It'd be an awkward and nontrivial bit of extra work to make this internal, though, given how the descriptor info is currently accessed by vkd3d_shader_parser_compile(). Unless I suppose it'd be acceptable to just keep the structures and everything the same but move the struct out of the function header? It'd be a bit awkward, mostly because we'd have a private element of enum vkd3d_shader_structure_type, but less painful. Or I could prototype a partial user of this in a reflection API, which broadly shouldn't be too hard anyway. The only really painful part of reflection, and the reason I haven't actually submitted anything yet, is designing the API for constant buffers. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/295#note_41297
I can see the value in avoiding exposing the API unless necessary. It'd be an awkward and nontrivial bit of extra work to make this internal, though, given how the descriptor info is currently accessed by vkd3d_shader_parser_compile().
Maybe not trivial, but it doesn't seem that hard? In particular, I imagine we can essentially just pass the new structure as optional parameter to scan_with_parser() and use it instead of "local_descriptor_info1" if present. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/295#note_41530
participants (3)
-
Henri Verbeet (＠hverbeet) -
Zebediah Figura
-
Zebediah Figura (＠zfigura)