[PATCH 0/1] MR465: vkd3d-shader/dxil: Fully initialize instruction data in sm6_parser_emit_extractval().

[Nikolay Sivov (＠nsivov)]

Noticed after test runner started crashing when tracing is enabled.

Signed-off-by: Nikolay Sivov

[PATCH 1/1] vkd3d-shader/dxil: Fully initialize instruction data in sm6_parser_emit_extractval().

[Nikolay Sivov]

From: Nikolay Sivov

Noticed after test runner started crashing when tracing is enabled.

Signed-off-by: Nikolay Sivov 
---
 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);

[Nikolay Sivov (＠nsivov)]

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.

[Henri Verbeet (＠hverbeet)]

> 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.

[PATCH 0/1] MR465: vkd3d-shader/dxil: Fully initialize instruction data in sm6_parser_emit_extractval(). - approved

[Henri Verbeet (＠hverbeet)]

This merge request was approved by Henri Verbeet.

[Nikolay Sivov (＠nsivov)]

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.

[Conor McCarthy (＠cmccarthy)]

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.

[PATCH 0/1] MR465: vkd3d-shader/dxil: Fully initialize instruction data in sm6_parser_emit_extractval(). - approved

[Giovanni Mascellani (＠giomasce)]

This merge request was approved by Giovanni Mascellani.