[PATCH 0/1] MR465: vkd3d-shader/dxil: Fully initialize instruction data in sm6_parser_emit_extractval().
Noticed after test runner started crashing when tracing is enabled. Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/465
From: Nikolay Sivov <nsivov(a)codeweavers.com> Noticed after test runner started crashing when tracing is enabled. Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- 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 2174ba52c..91be85401 100644 --- a/libs/vkd3d-shader/dxil.c +++ b/libs/vkd3d-shader/dxil.c @@ -3632,7 +3632,7 @@ static void sm6_parser_emit_extractval(struct sm6_parser *sm6, const struct dxil } dst->type = type; - ins->handler_idx = VKD3DSIH_MOV; + 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, src); -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/465
There are few places still that have NOP/INVALID opcode assigned directly, I left those out as they were not causing problems. If there is interested in using vsir_instruction_init() at all times, let me know. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/465#note_51995
There are few places still that have NOP/INVALID opcode assigned directly, I left those out as they were not causing problems. If there is interested in using vsir_instruction_init() at all times, let me know.
I think we generally should, yes. Note that there's a difference between creating a new NOP instruction and changing an existing instruction to NOP though; in the latter case we should use vkd3d_shader_instruction_make_nop() to preserve location information. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/465#note_52027
This merge request was approved by Henri Verbeet. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/465
On Sat Nov 11 19:01:41 2023 +0000, Henri Verbeet wrote:
There are few places still that have NOP/INVALID opcode assigned directly, I left those out as they were not causing problems. If there is interested in using vsir_instruction_init() at all times, let me know. I think we generally should, yes. Note that there's a difference between creating a new NOP instruction and changing an existing instruction to NOP though; in the latter case we should use vkd3d_shader_instruction_make_nop() to preserve location information. It's likely left uninitialized in a few places where NOP handler is set. We probably should call full init() at relatively top level, and then make_nop() if it's reset. That depends on whether location could change in between the two. For consistency, it makes sense to require handler_idx to be set only by a number of helpers.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/465#note_52029
On Sat Nov 11 19:01:41 2023 +0000, Nikolay Sivov wrote:
It's likely left uninitialized in a few places where NOP handler is set. We probably should call full init() at relatively top level, and then make_nop() if it's reset. That depends on whether location could change in between the two. For consistency, it makes sense to require handler_idx to be set only by a number of helpers. The DXIL parser never emits `NOP`, but in some places emission is skipped by setting `handler_idx` to `NOP`. This would probably be improved if we set a bool in `sm6_parser_function_init()` instead.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/465#note_52082
This merge request was approved by Giovanni Mascellani. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/465
participants (5)
-
Conor McCarthy (＠cmccarthy) -
Giovanni Mascellani (＠giomasce)
-
Henri Verbeet (＠hverbeet)
-
Nikolay Sivov
-
Nikolay Sivov (＠nsivov)