[PATCH v2 0/5] MR680: vkd3d-shader: Nearly get rid of vkd3d_shader_desc.
The only leftover field is SM1-specific, and I don't too much about how it's used, so I'm leaving it aside for now. It seems, however, that it could be moved directly to the parser (it seems to be only used for parsing and scanning). -- v2: vkd3d-shader: Remove the bytecode fields from vkd3d_shader_desc. vkd3d-shader/dxbc: Remove flag is_dxil. vkd3d-shader: Move shader signatures to vsir_program. https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/680
From: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- libs/vkd3d-shader/tpf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/vkd3d-shader/tpf.c b/libs/vkd3d-shader/tpf.c index c6cf1c951..9a5ab081b 100644 --- a/libs/vkd3d-shader/tpf.c +++ b/libs/vkd3d-shader/tpf.c @@ -2489,7 +2489,7 @@ static const struct vkd3d_shader_parser_ops shader_sm4_parser_ops = }; static bool shader_sm4_init(struct vkd3d_shader_sm4_parser *sm4, const uint32_t *byte_code, - size_t byte_code_size, const char *source_name, const struct shader_signature *output_signature, + size_t byte_code_size, const char *source_name, struct vkd3d_shader_message_context *message_context) { struct vkd3d_shader_version version; @@ -2655,7 +2655,7 @@ int vkd3d_shader_sm4_parser_create(const struct vkd3d_shader_compile_info *compi } if (!shader_sm4_init(sm4, shader_desc->byte_code, shader_desc->byte_code_size, - compile_info->source_name, &shader_desc->output_signature, message_context)) + compile_info->source_name, message_context)) { WARN("Failed to initialise shader parser.\n"); free_shader_desc(shader_desc); -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/680
From: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- libs/vkd3d-shader/dxbc.c | 13 ++++++++++--- libs/vkd3d-shader/dxil.c | 14 ++++++++++---- libs/vkd3d-shader/tpf.c | 15 ++++++++++++--- libs/vkd3d-shader/vkd3d_shader_private.h | 13 ++++++++++++- 4 files changed, 44 insertions(+), 11 deletions(-) diff --git a/libs/vkd3d-shader/dxbc.c b/libs/vkd3d-shader/dxbc.c index 37ebc73c0..861344d89 100644 --- a/libs/vkd3d-shader/dxbc.c +++ b/libs/vkd3d-shader/dxbc.c @@ -485,7 +485,7 @@ int shader_parse_input_signature(const struct vkd3d_shader_code *dxbc, static int shdr_handler(const struct vkd3d_shader_dxbc_section_desc *section, struct vkd3d_shader_message_context *message_context, void *context) { - struct vkd3d_shader_desc *desc = context; + struct dxbc_shader_desc *desc = context; int ret; switch (section->tag) @@ -550,6 +550,13 @@ static int shdr_handler(const struct vkd3d_shader_dxbc_section_desc *section, return VKD3D_OK; } +void free_dxbc_shader_desc(struct dxbc_shader_desc *desc) +{ + shader_signature_cleanup(&desc->input_signature); + shader_signature_cleanup(&desc->output_signature); + shader_signature_cleanup(&desc->patch_constant_signature); +} + void free_shader_desc(struct vkd3d_shader_desc *desc) { shader_signature_cleanup(&desc->input_signature); @@ -558,7 +565,7 @@ void free_shader_desc(struct vkd3d_shader_desc *desc) } int shader_extract_from_dxbc(const struct vkd3d_shader_code *dxbc, - struct vkd3d_shader_message_context *message_context, const char *source_name, struct vkd3d_shader_desc *desc) + struct vkd3d_shader_message_context *message_context, const char *source_name, struct dxbc_shader_desc *desc) { int ret; @@ -569,7 +576,7 @@ int shader_extract_from_dxbc(const struct vkd3d_shader_code *dxbc, if (ret < 0) { WARN("Failed to parse shader, vkd3d result %d.\n", ret); - free_shader_desc(desc); + free_dxbc_shader_desc(desc); } return ret; diff --git a/libs/vkd3d-shader/dxil.c b/libs/vkd3d-shader/dxil.c index 26a8a5c1c..be3be5fd7 100644 --- a/libs/vkd3d-shader/dxil.c +++ b/libs/vkd3d-shader/dxil.c @@ -8352,6 +8352,7 @@ int vkd3d_shader_sm6_parser_create(const struct vkd3d_shader_compile_info *compi struct vkd3d_shader_message_context *message_context, struct vkd3d_shader_parser **parser) { struct vkd3d_shader_desc *shader_desc; + struct dxbc_shader_desc dxbc_desc = {0}; uint32_t *byte_code = NULL; struct sm6_parser *sm6; int ret; @@ -8364,18 +8365,23 @@ int vkd3d_shader_sm6_parser_create(const struct vkd3d_shader_compile_info *compi return VKD3D_ERROR_OUT_OF_MEMORY; } - shader_desc = &sm6->p.shader_desc; - shader_desc->is_dxil = true; + dxbc_desc.is_dxil = true; if ((ret = shader_extract_from_dxbc(&compile_info->source, message_context, compile_info->source_name, - shader_desc)) < 0) + &dxbc_desc)) < 0) { WARN("Failed to extract shader, vkd3d result %d.\n", ret); vkd3d_free(sm6); return ret; } - sm6->p.shader_desc = *shader_desc; shader_desc = &sm6->p.shader_desc; + shader_desc->is_dxil = true; + shader_desc->byte_code = dxbc_desc.byte_code; + shader_desc->byte_code_size = dxbc_desc.byte_code_size; + shader_desc->input_signature = dxbc_desc.input_signature; + shader_desc->output_signature = dxbc_desc.output_signature; + shader_desc->patch_constant_signature = dxbc_desc.patch_constant_signature; + memset(&dxbc_desc, 0, sizeof(dxbc_desc)); if (((uintptr_t)shader_desc->byte_code & (VKD3D_DXBC_CHUNK_ALIGNMENT - 1))) { diff --git a/libs/vkd3d-shader/tpf.c b/libs/vkd3d-shader/tpf.c index 9a5ab081b..52cdd818b 100644 --- a/libs/vkd3d-shader/tpf.c +++ b/libs/vkd3d-shader/tpf.c @@ -2636,6 +2636,7 @@ int vkd3d_shader_sm4_parser_create(const struct vkd3d_shader_compile_info *compi struct vkd3d_shader_desc *shader_desc; struct vkd3d_shader_instruction *ins; struct vkd3d_shader_sm4_parser *sm4; + struct dxbc_shader_desc dxbc_desc = {0}; int ret; if (!(sm4 = vkd3d_calloc(1, sizeof(*sm4)))) @@ -2644,16 +2645,24 @@ int vkd3d_shader_sm4_parser_create(const struct vkd3d_shader_compile_info *compi return VKD3D_ERROR_OUT_OF_MEMORY; } - shader_desc = &sm4->p.shader_desc; - shader_desc->is_dxil = false; + dxbc_desc.is_dxil = false; if ((ret = shader_extract_from_dxbc(&compile_info->source, - message_context, compile_info->source_name, shader_desc)) < 0) + message_context, compile_info->source_name, &dxbc_desc)) < 0) { WARN("Failed to extract shader, vkd3d result %d.\n", ret); vkd3d_free(sm4); return ret; } + shader_desc = &sm4->p.shader_desc; + shader_desc->is_dxil = false; + shader_desc->byte_code = dxbc_desc.byte_code; + shader_desc->byte_code_size = dxbc_desc.byte_code_size; + shader_desc->input_signature = dxbc_desc.input_signature; + shader_desc->output_signature = dxbc_desc.output_signature; + shader_desc->patch_constant_signature = dxbc_desc.patch_constant_signature; + memset(&dxbc_desc, 0, sizeof(dxbc_desc)); + if (!shader_sm4_init(sm4, shader_desc->byte_code, shader_desc->byte_code_size, compile_info->source_name, message_context)) { diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index 4b322b95b..cdd8bbb10 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -1025,6 +1025,16 @@ struct signature_element *vsir_signature_find_element_for_reg(const struct shade unsigned int reg_idx, unsigned int write_mask); void shader_signature_cleanup(struct shader_signature *signature); +struct dxbc_shader_desc +{ + const uint32_t *byte_code; + size_t byte_code_size; + bool is_dxil; + struct shader_signature input_signature; + struct shader_signature output_signature; + struct shader_signature patch_constant_signature; +}; + struct vkd3d_shader_desc { const uint32_t *byte_code; @@ -1483,10 +1493,11 @@ int vkd3d_shader_sm4_parser_create(const struct vkd3d_shader_compile_info *compi int vkd3d_shader_sm6_parser_create(const struct vkd3d_shader_compile_info *compile_info, struct vkd3d_shader_message_context *message_context, struct vkd3d_shader_parser **parser); +void free_dxbc_shader_desc(struct dxbc_shader_desc *desc); void free_shader_desc(struct vkd3d_shader_desc *desc); int shader_extract_from_dxbc(const struct vkd3d_shader_code *dxbc, - struct vkd3d_shader_message_context *message_context, const char *source_name, struct vkd3d_shader_desc *desc); + struct vkd3d_shader_message_context *message_context, const char *source_name, struct dxbc_shader_desc *desc); int shader_parse_input_signature(const struct vkd3d_shader_code *dxbc, struct vkd3d_shader_message_context *message_context, struct shader_signature *signature); -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/680
From: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- libs/vkd3d-shader/d3dbc.c | 9 +++--- libs/vkd3d-shader/dxbc.c | 7 ----- libs/vkd3d-shader/dxil.c | 37 ++++++++++++------------ libs/vkd3d-shader/ir.c | 19 +++++++----- libs/vkd3d-shader/spirv.c | 27 +++++++++-------- libs/vkd3d-shader/tpf.c | 36 +++++++++++------------ libs/vkd3d-shader/vkd3d_shader_main.c | 6 ++-- libs/vkd3d-shader/vkd3d_shader_private.h | 8 ++--- 8 files changed, 72 insertions(+), 77 deletions(-) diff --git a/libs/vkd3d-shader/d3dbc.c b/libs/vkd3d-shader/d3dbc.c index 27f5c8104..8e34a0b82 100644 --- a/libs/vkd3d-shader/d3dbc.c +++ b/libs/vkd3d-shader/d3dbc.c @@ -541,9 +541,9 @@ static bool add_signature_element(struct vkd3d_shader_sm1_parser *sm1, bool outp struct signature_element *element; if (output) - signature = &sm1->p.shader_desc.output_signature; + signature = &sm1->p.program.output_signature; else - signature = &sm1->p.shader_desc.input_signature; + signature = &sm1->p.program.input_signature; if ((element = find_signature_element(signature, name, index))) { @@ -581,9 +581,9 @@ static void add_signature_mask(struct vkd3d_shader_sm1_parser *sm1, bool output, struct signature_element *element; if (output) - signature = &sm1->p.shader_desc.output_signature; + signature = &sm1->p.program.output_signature; else - signature = &sm1->p.shader_desc.input_signature; + signature = &sm1->p.program.input_signature; if (!(element = find_signature_element_by_register_index(signature, register_index))) { @@ -886,7 +886,6 @@ static void shader_sm1_destroy(struct vkd3d_shader_parser *parser) struct vkd3d_shader_sm1_parser *sm1 = vkd3d_shader_sm1_parser(parser); vsir_program_cleanup(&parser->program); - free_shader_desc(&sm1->p.shader_desc); vkd3d_free(sm1); } diff --git a/libs/vkd3d-shader/dxbc.c b/libs/vkd3d-shader/dxbc.c index 861344d89..10ce2fb39 100644 --- a/libs/vkd3d-shader/dxbc.c +++ b/libs/vkd3d-shader/dxbc.c @@ -557,13 +557,6 @@ void free_dxbc_shader_desc(struct dxbc_shader_desc *desc) shader_signature_cleanup(&desc->patch_constant_signature); } -void free_shader_desc(struct vkd3d_shader_desc *desc) -{ - shader_signature_cleanup(&desc->input_signature); - shader_signature_cleanup(&desc->output_signature); - shader_signature_cleanup(&desc->patch_constant_signature); -} - int shader_extract_from_dxbc(const struct vkd3d_shader_code *dxbc, struct vkd3d_shader_message_context *message_context, const char *source_name, struct dxbc_shader_desc *desc) { diff --git a/libs/vkd3d-shader/dxil.c b/libs/vkd3d-shader/dxil.c index be3be5fd7..faa44fdc4 100644 --- a/libs/vkd3d-shader/dxil.c +++ b/libs/vkd3d-shader/dxil.c @@ -4182,7 +4182,7 @@ static void sm6_parser_emit_dx_load_input(struct sm6_parser *sm6, enum dx_intrin vsir_instruction_init(ins, &sm6->p.location, VKD3DSIH_MOV); - signature = &sm6->p.shader_desc.input_signature; + signature = &sm6->p.program.input_signature; if (row_index >= signature->element_count) { WARN("Invalid row index %u.\n", row_index); @@ -4572,7 +4572,7 @@ static void sm6_parser_emit_dx_store_output(struct sm6_parser *sm6, enum dx_intr row_index = sm6_value_get_constant_uint(operands[0]); column_index = sm6_value_get_constant_uint(operands[2]); - signature = &sm6->p.shader_desc.output_signature; + signature = &sm6->p.program.output_signature; if (row_index >= signature->element_count) { WARN("Invalid row index %u.\n", row_index); @@ -7752,19 +7752,19 @@ static enum vkd3d_result sm6_parser_signatures_init(struct sm6_parser *sm6, cons } if (m->u.node->operand_count && (ret = sm6_parser_read_signature(sm6, m->u.node->operands[0], - &sm6->p.shader_desc.input_signature)) < 0) + &sm6->p.program.input_signature)) < 0) { return ret; } if (m->u.node->operand_count > 1 && (ret = sm6_parser_read_signature(sm6, m->u.node->operands[1], - &sm6->p.shader_desc.output_signature)) < 0) + &sm6->p.program.output_signature)) < 0) { return ret; } /* TODO: patch constant signature in operand 2. */ - sm6_parser_init_input_signature(sm6, &sm6->p.shader_desc.input_signature); - sm6_parser_init_output_signature(sm6, &sm6->p.shader_desc.output_signature); + sm6_parser_init_input_signature(sm6, &sm6->p.program.input_signature); + sm6_parser_init_output_signature(sm6, &sm6->p.program.output_signature); return VKD3D_OK; } @@ -8062,7 +8062,6 @@ static void sm6_parser_destroy(struct vkd3d_shader_parser *parser) sm6_parser_metadata_cleanup(sm6); vkd3d_free(sm6->descriptors); vkd3d_free(sm6->values); - free_shader_desc(&parser->shader_desc); vkd3d_free(sm6); } @@ -8080,15 +8079,16 @@ static struct sm6_function *sm6_parser_get_function(const struct sm6_parser *sm6 return NULL; } -static enum vkd3d_result sm6_parser_init(struct sm6_parser *sm6, const uint32_t *byte_code, size_t byte_code_size, - const char *source_name, struct vkd3d_shader_message_context *message_context) +static enum vkd3d_result sm6_parser_init(struct sm6_parser *sm6, const char *source_name, + struct vkd3d_shader_message_context *message_context, struct dxbc_shader_desc *dxbc_desc) { - const struct shader_signature *output_signature = &sm6->p.shader_desc.output_signature; - const struct shader_signature *input_signature = &sm6->p.shader_desc.input_signature; + const struct shader_signature *output_signature = &sm6->p.program.output_signature; + const struct shader_signature *input_signature = &sm6->p.program.input_signature; + size_t count, length, function_count, byte_code_size = dxbc_desc->byte_code_size; const struct vkd3d_shader_location location = {.source_name = source_name}; uint32_t version_token, dxil_version, token_count, magic; + const uint32_t *byte_code = dxbc_desc->byte_code; unsigned int chunk_offset, chunk_size; - size_t count, length, function_count; enum bitcode_block_abbreviation abbr; struct vkd3d_shader_version version; struct dxil_block *block; @@ -8181,6 +8181,11 @@ static enum vkd3d_result sm6_parser_init(struct sm6_parser *sm6, const uint32_t sm6->ptr = &sm6->start[1]; sm6->bitpos = 2; + sm6->p.program.input_signature = dxbc_desc->input_signature; + sm6->p.program.output_signature = dxbc_desc->output_signature; + sm6->p.program.patch_constant_signature = dxbc_desc->patch_constant_signature; + memset(dxbc_desc, 0, sizeof(*dxbc_desc)); + block = &sm6->root_block; if ((ret = dxil_block_init(block, NULL, sm6)) < 0) { @@ -8378,10 +8383,6 @@ int vkd3d_shader_sm6_parser_create(const struct vkd3d_shader_compile_info *compi shader_desc->is_dxil = true; shader_desc->byte_code = dxbc_desc.byte_code; shader_desc->byte_code_size = dxbc_desc.byte_code_size; - shader_desc->input_signature = dxbc_desc.input_signature; - shader_desc->output_signature = dxbc_desc.output_signature; - shader_desc->patch_constant_signature = dxbc_desc.patch_constant_signature; - memset(&dxbc_desc, 0, sizeof(dxbc_desc)); if (((uintptr_t)shader_desc->byte_code & (VKD3D_DXBC_CHUNK_ALIGNMENT - 1))) { @@ -8391,10 +8392,10 @@ int vkd3d_shader_sm6_parser_create(const struct vkd3d_shader_compile_info *compi ERR("Failed to allocate aligned chunk. Unaligned access will occur.\n"); else memcpy(byte_code, shader_desc->byte_code, shader_desc->byte_code_size); + dxbc_desc.byte_code = byte_code; } - ret = sm6_parser_init(sm6, byte_code ? byte_code : shader_desc->byte_code, shader_desc->byte_code_size, - compile_info->source_name, message_context); + ret = sm6_parser_init(sm6, compile_info->source_name, message_context, &dxbc_desc); vkd3d_free(byte_code); if (!sm6->p.failed && ret >= 0) diff --git a/libs/vkd3d-shader/ir.c b/libs/vkd3d-shader/ir.c index f0bd85338..7ace168a0 100644 --- a/libs/vkd3d-shader/ir.c +++ b/libs/vkd3d-shader/ir.c @@ -32,6 +32,9 @@ void vsir_program_cleanup(struct vsir_program *program) vkd3d_free((void *)program->block_names[i]); vkd3d_free(program->block_names); shader_instruction_array_destroy(&program->instructions); + shader_signature_cleanup(&program->input_signature); + shader_signature_cleanup(&program->output_signature); + shader_signature_cleanup(&program->patch_constant_signature); } static inline bool shader_register_is_phase_instance_id(const struct vkd3d_shader_register *reg) @@ -230,7 +233,7 @@ static const struct vkd3d_shader_varying_map *find_varying_map( static enum vkd3d_result remap_output_signature(struct vkd3d_shader_parser *parser, const struct vkd3d_shader_compile_info *compile_info) { - struct shader_signature *signature = &parser->shader_desc.output_signature; + struct shader_signature *signature = &parser->program.output_signature; const struct vkd3d_shader_varying_map_info *varying_map; unsigned int i; @@ -1394,9 +1397,9 @@ static enum vkd3d_result shader_normalise_io_registers(struct vkd3d_shader_parse normaliser.phase = VKD3DSIH_INVALID; normaliser.shader_type = program->shader_version.type; normaliser.major = program->shader_version.major; - normaliser.input_signature = &parser->shader_desc.input_signature; - normaliser.output_signature = &parser->shader_desc.output_signature; - normaliser.patch_constant_signature = &parser->shader_desc.patch_constant_signature; + normaliser.input_signature = &parser->program.input_signature; + normaliser.output_signature = &parser->program.output_signature; + normaliser.patch_constant_signature = &parser->program.patch_constant_signature; for (i = 0, has_control_point_phase = false; i < program->instructions.count; ++i) { @@ -1439,9 +1442,9 @@ static enum vkd3d_result shader_normalise_io_registers(struct vkd3d_shader_parse } } - if (!shader_signature_merge(&parser->shader_desc.input_signature, normaliser.input_range_map, false) - || !shader_signature_merge(&parser->shader_desc.output_signature, normaliser.output_range_map, false) - || !shader_signature_merge(&parser->shader_desc.patch_constant_signature, normaliser.pc_range_map, true)) + if (!shader_signature_merge(&parser->program.input_signature, normaliser.input_range_map, false) + || !shader_signature_merge(&parser->program.output_signature, normaliser.output_range_map, false) + || !shader_signature_merge(&parser->program.patch_constant_signature, normaliser.pc_range_map, true)) { program->instructions = normaliser.instructions; return VKD3D_ERROR_OUT_OF_MEMORY; @@ -3370,7 +3373,7 @@ enum vkd3d_result vkd3d_shader_normalise(struct vkd3d_shader_parser *parser, return result; if ((result = instruction_array_normalise_hull_shader_control_point_io(instructions, - &parser->shader_desc.input_signature)) < 0) + &parser->program.input_signature)) < 0) return result; } diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c index a3baeea75..ea2d04dcd 100644 --- a/libs/vkd3d-shader/spirv.c +++ b/libs/vkd3d-shader/spirv.c @@ -2427,14 +2427,14 @@ static void spirv_compiler_destroy(struct spirv_compiler *compiler) vkd3d_free(compiler); } -static struct spirv_compiler *spirv_compiler_create(const struct vkd3d_shader_version *shader_version, - struct vkd3d_shader_desc *shader_desc, const struct vkd3d_shader_compile_info *compile_info, +static struct spirv_compiler *spirv_compiler_create(const struct vsir_program *program, + const struct vkd3d_shader_compile_info *compile_info, const struct vkd3d_shader_scan_descriptor_info1 *scan_descriptor_info, struct vkd3d_shader_message_context *message_context, const struct vkd3d_shader_location *location, uint64_t config_flags) { - const struct shader_signature *patch_constant_signature = &shader_desc->patch_constant_signature; - const struct shader_signature *output_signature = &shader_desc->output_signature; + const struct shader_signature *patch_constant_signature = &program->patch_constant_signature; + const struct shader_signature *output_signature = &program->output_signature; const struct vkd3d_shader_interface_info *shader_interface; const struct vkd3d_shader_descriptor_offset_info *offset_info; const struct vkd3d_shader_spirv_target_info *target_info; @@ -2545,7 +2545,7 @@ static struct spirv_compiler *spirv_compiler_create(const struct vkd3d_shader_ve rb_init(&compiler->symbol_table, vkd3d_symbol_compare); - compiler->shader_type = shader_version->type; + compiler->shader_type = program->shader_version.type; if ((shader_interface = vkd3d_find_struct(compile_info->next, INTERFACE_INFO))) { @@ -9899,7 +9899,6 @@ static int spirv_compiler_generate_spirv(struct spirv_compiler *compiler, const struct vkd3d_shader_spirv_target_info *info = compiler->spirv_target_info; const struct vkd3d_shader_spirv_domain_shader_target_info *ds_info; struct vkd3d_spirv_builder *builder = &compiler->spirv_builder; - struct vkd3d_shader_desc *shader_desc = &parser->shader_desc; struct vkd3d_shader_instruction_array instructions; struct vsir_program *program = &parser->program; enum vkd3d_result result = VKD3D_OK; @@ -9924,12 +9923,12 @@ static int spirv_compiler_generate_spirv(struct spirv_compiler *compiler, instructions = program->instructions; memset(&program->instructions, 0, sizeof(program->instructions)); - compiler->input_signature = shader_desc->input_signature; - compiler->output_signature = shader_desc->output_signature; - compiler->patch_constant_signature = shader_desc->patch_constant_signature; - memset(&shader_desc->input_signature, 0, sizeof(shader_desc->input_signature)); - memset(&shader_desc->output_signature, 0, sizeof(shader_desc->output_signature)); - memset(&shader_desc->patch_constant_signature, 0, sizeof(shader_desc->patch_constant_signature)); + compiler->input_signature = program->input_signature; + compiler->output_signature = program->output_signature; + compiler->patch_constant_signature = program->patch_constant_signature; + memset(&program->input_signature, 0, sizeof(program->input_signature)); + memset(&program->output_signature, 0, sizeof(program->output_signature)); + memset(&program->patch_constant_signature, 0, sizeof(program->patch_constant_signature)); compiler->use_vocp = program->use_vocp; compiler->block_names = program->block_names; compiler->block_name_count = program->block_name_count; @@ -10036,8 +10035,8 @@ int spirv_compile(struct vkd3d_shader_parser *parser, struct spirv_compiler *spirv_compiler; int ret; - if (!(spirv_compiler = spirv_compiler_create(&parser->program.shader_version, &parser->shader_desc, - compile_info, scan_descriptor_info, message_context, &parser->location, parser->config_flags))) + if (!(spirv_compiler = spirv_compiler_create(&parser->program, compile_info, + scan_descriptor_info, message_context, &parser->location, parser->config_flags))) { ERR("Failed to create SPIR-V compiler.\n"); return VKD3D_ERROR; diff --git a/libs/vkd3d-shader/tpf.c b/libs/vkd3d-shader/tpf.c index 52cdd818b..46fca594c 100644 --- a/libs/vkd3d-shader/tpf.c +++ b/libs/vkd3d-shader/tpf.c @@ -939,32 +939,32 @@ static void shader_sm4_read_dcl_index_range(struct vkd3d_shader_instruction *ins case VKD3DSPR_INCONTROLPOINT: io_masks = priv->input_register_masks; ranges = &priv->input_index_ranges; - signature = &priv->p.shader_desc.input_signature; + signature = &priv->p.program.input_signature; break; case VKD3DSPR_OUTPUT: if (sm4_parser_is_in_fork_or_join_phase(priv)) { io_masks = priv->patch_constant_register_masks; ranges = &priv->patch_constant_index_ranges; - signature = &priv->p.shader_desc.patch_constant_signature; + signature = &priv->p.program.patch_constant_signature; } else { io_masks = priv->output_register_masks; ranges = &priv->output_index_ranges; - signature = &priv->p.shader_desc.output_signature; + signature = &priv->p.program.output_signature; } break; case VKD3DSPR_COLOROUT: case VKD3DSPR_OUTCONTROLPOINT: io_masks = priv->output_register_masks; ranges = &priv->output_index_ranges; - signature = &priv->p.shader_desc.output_signature; + signature = &priv->p.program.output_signature; break; case VKD3DSPR_PATCHCONST: io_masks = priv->patch_constant_register_masks; ranges = &priv->patch_constant_index_ranges; - signature = &priv->p.shader_desc.patch_constant_signature; + signature = &priv->p.program.patch_constant_signature; break; default: @@ -1098,7 +1098,7 @@ static void shader_sm4_read_dcl_input_ps(struct vkd3d_shader_instruction *ins, u if (shader_sm4_read_dst_param(priv, &tokens, &tokens[token_count], VKD3D_DATA_FLOAT, dst)) { struct signature_element *e = vsir_signature_find_element_for_reg( - &priv->p.shader_desc.input_signature, dst->reg.idx[dst->reg.idx_count - 1].offset, dst->write_mask); + &priv->p.program.input_signature, dst->reg.idx[dst->reg.idx_count - 1].offset, dst->write_mask); e->interpolation_mode = ins->flags; } @@ -1113,7 +1113,7 @@ static void shader_sm4_read_dcl_input_ps_siv(struct vkd3d_shader_instruction *in if (shader_sm4_read_dst_param(priv, &tokens, &tokens[token_count], VKD3D_DATA_FLOAT, dst)) { struct signature_element *e = vsir_signature_find_element_for_reg( - &priv->p.shader_desc.input_signature, dst->reg.idx[dst->reg.idx_count - 1].offset, dst->write_mask); + &priv->p.program.input_signature, dst->reg.idx[dst->reg.idx_count - 1].offset, dst->write_mask); e->interpolation_mode = ins->flags; } @@ -1733,7 +1733,6 @@ static void shader_sm4_destroy(struct vkd3d_shader_parser *parser) struct vkd3d_shader_sm4_parser *sm4 = vkd3d_shader_sm4_parser(parser); vsir_program_cleanup(&parser->program); - free_shader_desc(&parser->shader_desc); vkd3d_free(sm4); } @@ -2658,31 +2657,32 @@ int vkd3d_shader_sm4_parser_create(const struct vkd3d_shader_compile_info *compi shader_desc->is_dxil = false; shader_desc->byte_code = dxbc_desc.byte_code; shader_desc->byte_code_size = dxbc_desc.byte_code_size; - shader_desc->input_signature = dxbc_desc.input_signature; - shader_desc->output_signature = dxbc_desc.output_signature; - shader_desc->patch_constant_signature = dxbc_desc.patch_constant_signature; - memset(&dxbc_desc, 0, sizeof(dxbc_desc)); if (!shader_sm4_init(sm4, shader_desc->byte_code, shader_desc->byte_code_size, compile_info->source_name, message_context)) { WARN("Failed to initialise shader parser.\n"); - free_shader_desc(shader_desc); + free_dxbc_shader_desc(&dxbc_desc); vkd3d_free(sm4); return VKD3D_ERROR_INVALID_ARGUMENT; } + sm4->p.program.input_signature = dxbc_desc.input_signature; + sm4->p.program.output_signature = dxbc_desc.output_signature; + sm4->p.program.patch_constant_signature = dxbc_desc.patch_constant_signature; + memset(&dxbc_desc, 0, sizeof(dxbc_desc)); + /* DXBC stores used masks inverted for output signatures, for some reason. * We return them un-inverted. */ - uninvert_used_masks(&shader_desc->output_signature); + uninvert_used_masks(&sm4->p.program.output_signature); if (sm4->p.program.shader_version.type == VKD3D_SHADER_TYPE_HULL) - uninvert_used_masks(&shader_desc->patch_constant_signature); + uninvert_used_masks(&sm4->p.program.patch_constant_signature); - if (!shader_sm4_parser_validate_signature(sm4, &shader_desc->input_signature, + if (!shader_sm4_parser_validate_signature(sm4, &sm4->p.program.input_signature, sm4->input_register_masks, "Input") - || !shader_sm4_parser_validate_signature(sm4, &shader_desc->output_signature, + || !shader_sm4_parser_validate_signature(sm4, &sm4->p.program.output_signature, sm4->output_register_masks, "Output") - || !shader_sm4_parser_validate_signature(sm4, &shader_desc->patch_constant_signature, + || !shader_sm4_parser_validate_signature(sm4, &sm4->p.program.patch_constant_signature, sm4->patch_constant_register_masks, "Patch constant")) { shader_sm4_destroy(&sm4->p); diff --git a/libs/vkd3d-shader/vkd3d_shader_main.c b/libs/vkd3d-shader/vkd3d_shader_main.c index 462a5c25e..f63a213f0 100644 --- a/libs/vkd3d-shader/vkd3d_shader_main.c +++ b/libs/vkd3d-shader/vkd3d_shader_main.c @@ -1440,11 +1440,11 @@ static int scan_with_parser(const struct vkd3d_shader_compile_info *compile_info if (!ret && signature_info) { - if (!vkd3d_shader_signature_from_shader_signature(&signature_info->input, &parser->shader_desc.input_signature) + if (!vkd3d_shader_signature_from_shader_signature(&signature_info->input, &parser->program.input_signature) || !vkd3d_shader_signature_from_shader_signature(&signature_info->output, - &parser->shader_desc.output_signature) + &parser->program.output_signature) || !vkd3d_shader_signature_from_shader_signature(&signature_info->patch_constant, - &parser->shader_desc.patch_constant_signature)) + &parser->program.patch_constant_signature)) { ret = VKD3D_ERROR_OUT_OF_MEMORY; } diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index cdd8bbb10..aa91f5489 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -1040,9 +1040,6 @@ struct vkd3d_shader_desc const uint32_t *byte_code; size_t byte_code_size; bool is_dxil; - struct shader_signature input_signature; - struct shader_signature output_signature; - struct shader_signature patch_constant_signature; struct { @@ -1300,6 +1297,10 @@ struct vsir_program struct vkd3d_shader_version shader_version; struct vkd3d_shader_instruction_array instructions; + struct shader_signature input_signature; + struct shader_signature output_signature; + struct shader_signature patch_constant_signature; + unsigned int input_control_point_count, output_control_point_count; unsigned int block_count; unsigned int temp_count; @@ -1494,7 +1495,6 @@ int vkd3d_shader_sm6_parser_create(const struct vkd3d_shader_compile_info *compi struct vkd3d_shader_message_context *message_context, struct vkd3d_shader_parser **parser); void free_dxbc_shader_desc(struct dxbc_shader_desc *desc); -void free_shader_desc(struct vkd3d_shader_desc *desc); int shader_extract_from_dxbc(const struct vkd3d_shader_code *dxbc, struct vkd3d_shader_message_context *message_context, const char *source_name, struct dxbc_shader_desc *desc); -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/680
From: Giovanni Mascellani <gmascellani(a)codeweavers.com> The full shader profile is already available in vsir_program. --- libs/vkd3d-shader/dxil.c | 1 - libs/vkd3d-shader/ir.c | 2 +- libs/vkd3d-shader/tpf.c | 1 - libs/vkd3d-shader/vkd3d_shader_private.h | 1 - 4 files changed, 1 insertion(+), 4 deletions(-) diff --git a/libs/vkd3d-shader/dxil.c b/libs/vkd3d-shader/dxil.c index faa44fdc4..1f37259ee 100644 --- a/libs/vkd3d-shader/dxil.c +++ b/libs/vkd3d-shader/dxil.c @@ -8380,7 +8380,6 @@ int vkd3d_shader_sm6_parser_create(const struct vkd3d_shader_compile_info *compi } shader_desc = &sm6->p.shader_desc; - shader_desc->is_dxil = true; shader_desc->byte_code = dxbc_desc.byte_code; shader_desc->byte_code_size = dxbc_desc.byte_code_size; diff --git a/libs/vkd3d-shader/ir.c b/libs/vkd3d-shader/ir.c index 7ace168a0..d27ffb39d 100644 --- a/libs/vkd3d-shader/ir.c +++ b/libs/vkd3d-shader/ir.c @@ -3336,7 +3336,7 @@ enum vkd3d_result vkd3d_shader_normalise(struct vkd3d_shader_parser *parser, if ((result = instruction_array_lower_texkills(parser)) < 0) return result; - if (parser->shader_desc.is_dxil) + if (parser->program.shader_version.major >= 6) { struct vsir_cfg cfg; diff --git a/libs/vkd3d-shader/tpf.c b/libs/vkd3d-shader/tpf.c index 46fca594c..2f5726292 100644 --- a/libs/vkd3d-shader/tpf.c +++ b/libs/vkd3d-shader/tpf.c @@ -2654,7 +2654,6 @@ int vkd3d_shader_sm4_parser_create(const struct vkd3d_shader_compile_info *compi } shader_desc = &sm4->p.shader_desc; - shader_desc->is_dxil = false; shader_desc->byte_code = dxbc_desc.byte_code; shader_desc->byte_code_size = dxbc_desc.byte_code_size; diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index aa91f5489..8db1fdd33 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -1039,7 +1039,6 @@ struct vkd3d_shader_desc { const uint32_t *byte_code; size_t byte_code_size; - bool is_dxil; struct { -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/680
From: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- libs/vkd3d-shader/d3dbc.c | 4 ---- libs/vkd3d-shader/dxil.c | 11 +++-------- libs/vkd3d-shader/tpf.c | 7 +------ libs/vkd3d-shader/vkd3d_shader_private.h | 3 --- 4 files changed, 4 insertions(+), 21 deletions(-) diff --git a/libs/vkd3d-shader/d3dbc.c b/libs/vkd3d-shader/d3dbc.c index 8e34a0b82..3293b905a 100644 --- a/libs/vkd3d-shader/d3dbc.c +++ b/libs/vkd3d-shader/d3dbc.c @@ -1236,7 +1236,6 @@ static enum vkd3d_result shader_sm1_init(struct vkd3d_shader_sm1_parser *sm1, const struct vkd3d_shader_location location = {.source_name = compile_info->source_name}; const uint32_t *code = compile_info->source.code; size_t code_size = compile_info->source.size; - struct vkd3d_shader_desc *shader_desc; struct vkd3d_shader_version version; uint16_t shader_type; size_t token_count; @@ -1289,9 +1288,6 @@ static enum vkd3d_result shader_sm1_init(struct vkd3d_shader_sm1_parser *sm1, if (!vkd3d_shader_parser_init(&sm1->p, message_context, compile_info->source_name, &version, &shader_sm1_parser_ops, code_size != ~(size_t)0 ? token_count / 4u + 4 : 16)) return VKD3D_ERROR_OUT_OF_MEMORY; - shader_desc = &sm1->p.shader_desc; - shader_desc->byte_code = code; - shader_desc->byte_code_size = code_size; sm1->ptr = sm1->start; return VKD3D_OK; diff --git a/libs/vkd3d-shader/dxil.c b/libs/vkd3d-shader/dxil.c index 1f37259ee..ef02ae5cd 100644 --- a/libs/vkd3d-shader/dxil.c +++ b/libs/vkd3d-shader/dxil.c @@ -8356,7 +8356,6 @@ static enum vkd3d_result sm6_parser_init(struct sm6_parser *sm6, const char *sou int vkd3d_shader_sm6_parser_create(const struct vkd3d_shader_compile_info *compile_info, struct vkd3d_shader_message_context *message_context, struct vkd3d_shader_parser **parser) { - struct vkd3d_shader_desc *shader_desc; struct dxbc_shader_desc dxbc_desc = {0}; uint32_t *byte_code = NULL; struct sm6_parser *sm6; @@ -8379,18 +8378,14 @@ int vkd3d_shader_sm6_parser_create(const struct vkd3d_shader_compile_info *compi return ret; } - shader_desc = &sm6->p.shader_desc; - shader_desc->byte_code = dxbc_desc.byte_code; - shader_desc->byte_code_size = dxbc_desc.byte_code_size; - - if (((uintptr_t)shader_desc->byte_code & (VKD3D_DXBC_CHUNK_ALIGNMENT - 1))) + if (((uintptr_t)dxbc_desc.byte_code & (VKD3D_DXBC_CHUNK_ALIGNMENT - 1))) { /* LLVM bitcode should be 32-bit aligned, but before dxc v1.7.2207 this was not always the case in the DXBC * container due to missing padding after signature names. Get an aligned copy to prevent unaligned access. */ - if (!(byte_code = vkd3d_malloc(align(shader_desc->byte_code_size, VKD3D_DXBC_CHUNK_ALIGNMENT)))) + if (!(byte_code = vkd3d_malloc(align(dxbc_desc.byte_code_size, VKD3D_DXBC_CHUNK_ALIGNMENT)))) ERR("Failed to allocate aligned chunk. Unaligned access will occur.\n"); else - memcpy(byte_code, shader_desc->byte_code, shader_desc->byte_code_size); + memcpy(byte_code, dxbc_desc.byte_code, dxbc_desc.byte_code_size); dxbc_desc.byte_code = byte_code; } diff --git a/libs/vkd3d-shader/tpf.c b/libs/vkd3d-shader/tpf.c index 2f5726292..e8a74a437 100644 --- a/libs/vkd3d-shader/tpf.c +++ b/libs/vkd3d-shader/tpf.c @@ -2632,7 +2632,6 @@ int vkd3d_shader_sm4_parser_create(const struct vkd3d_shader_compile_info *compi struct vkd3d_shader_message_context *message_context, struct vkd3d_shader_parser **parser) { struct vkd3d_shader_instruction_array *instructions; - struct vkd3d_shader_desc *shader_desc; struct vkd3d_shader_instruction *ins; struct vkd3d_shader_sm4_parser *sm4; struct dxbc_shader_desc dxbc_desc = {0}; @@ -2653,11 +2652,7 @@ int vkd3d_shader_sm4_parser_create(const struct vkd3d_shader_compile_info *compi return ret; } - shader_desc = &sm4->p.shader_desc; - shader_desc->byte_code = dxbc_desc.byte_code; - shader_desc->byte_code_size = dxbc_desc.byte_code_size; - - if (!shader_sm4_init(sm4, shader_desc->byte_code, shader_desc->byte_code_size, + if (!shader_sm4_init(sm4, dxbc_desc.byte_code, dxbc_desc.byte_code_size, compile_info->source_name, message_context)) { WARN("Failed to initialise shader parser.\n"); diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index 8db1fdd33..3422d5be6 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -1037,9 +1037,6 @@ struct dxbc_shader_desc struct vkd3d_shader_desc { - const uint32_t *byte_code; - size_t byte_code_size; - struct { uint32_t used, external; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/680
Francisco Casas (@fcasas) commented about libs/vkd3d-shader/dxil.c:
ERR("Failed to allocate aligned chunk. Unaligned access will occur.\n"); else memcpy(byte_code, dxbc_desc.byte_code, dxbc_desc.byte_code_size); + dxbc_desc.byte_code = byte_code;
Shouldn't we free the original `dxbc_desc.byte_code` before replacing it? -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/680#note_63788
On Wed Mar 6 23:40:34 2024 +0000, Francisco Casas wrote:
Shouldn't we free the original `dxbc_desc.byte_code` before replacing it? Also, it seems that the assignment should only happen inside the previous "else".
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/680#note_63789
Francisco Casas (@fcasas) commented about libs/vkd3d-shader/ir.c:
normaliser.phase = VKD3DSIH_INVALID; normaliser.shader_type = program->shader_version.type; normaliser.major = program->shader_version.major; - normaliser.input_signature = &parser->shader_desc.input_signature; - normaliser.output_signature = &parser->shader_desc.output_signature; - normaliser.patch_constant_signature = &parser->shader_desc.patch_constant_signature; + normaliser.input_signature = &parser->program.input_signature; + normaliser.output_signature = &parser->program.output_signature; + normaliser.patch_constant_signature = &parser->program.patch_constant_signature;
nitpick: it is possible to use the local variable `program` here, set to `&parser->program` some lines above. Same in the following change. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/680#note_63799
Francisco Casas (@fcasas) commented about libs/vkd3d-shader/dxil.c:
return NULL; }
-static enum vkd3d_result sm6_parser_init(struct sm6_parser *sm6, const uint32_t *byte_code, size_t byte_code_size, - const char *source_name, struct vkd3d_shader_message_context *message_context) +static enum vkd3d_result sm6_parser_init(struct sm6_parser *sm6, const char *source_name, + struct vkd3d_shader_message_context *message_context, struct dxbc_shader_desc *dxbc_desc)
I am not fond of passing `dxbc_desc` as a parameter and use it far later to initialize the signatures in `sm6->p.program`. It may be hard to see from outside the function that `dxcb_desc` will be lending its signatures to `sm6->p.program` and be memset to 0. Also, any error before `dxbc_desc` lends the signatures to `sm6->p.program` leaks these signatures. How about creating `dxbc_desc` inside `sm6_parser_init()` right before lending the signatures? This would require passing the whole `compile_info` as a parameter and also move the 32-bit alignment there. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/680#note_63800
Shouldn't we free the original `dxbc_desc.byte_code` before replacing it?
No, `dxbc_desc` doesn't own its buffer. It's just a pointer into the original source. For the same reason `byte_code` has to be owned by `vkd3d_shader_sm6_parser_create()` and therefore manually freed.
Also, it seems that the assignment should only happen inside the previous "else".
More importantly, I'd say that failure to allocate should be a hard error. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/680#note_63815
participants (3)
-
Francisco Casas (＠fcasas) -
Giovanni Mascellani
-
Giovanni Mascellani (＠giomasce)