[PATCH 0/2] MR257: vkd3d-shader: Two valgrind fixes.
From: Zebediah Figura <zfigura(a)codeweavers.com> We emit one source with multiple components. --- libs/vkd3d-shader/d3dbc.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libs/vkd3d-shader/d3dbc.c b/libs/vkd3d-shader/d3dbc.c index d2a4666a..772e4ee1 100644 --- a/libs/vkd3d-shader/d3dbc.c +++ b/libs/vkd3d-shader/d3dbc.c @@ -260,9 +260,9 @@ static const struct vkd3d_sm1_opcode_info vs_opcode_table[] = /* Declarations */ {VKD3D_SM1_OP_DCL, 0, 2, VKD3DSIH_DCL}, /* Constant definitions */ - {VKD3D_SM1_OP_DEF, 1, 4, VKD3DSIH_DEF}, + {VKD3D_SM1_OP_DEF, 1, 1, VKD3DSIH_DEF}, {VKD3D_SM1_OP_DEFB, 1, 1, VKD3DSIH_DEFB}, - {VKD3D_SM1_OP_DEFI, 1, 4, VKD3DSIH_DEFI}, + {VKD3D_SM1_OP_DEFI, 1, 1, VKD3DSIH_DEFI}, /* Control flow */ {VKD3D_SM1_OP_REP, 0, 1, VKD3DSIH_REP, {2, 0}, {~0u, ~0u}}, {VKD3D_SM1_OP_ENDREP, 0, 0, VKD3DSIH_ENDREP, {2, 0}, {~0u, ~0u}}, @@ -327,9 +327,9 @@ static const struct vkd3d_sm1_opcode_info ps_opcode_table[] = /* Declarations */ {VKD3D_SM1_OP_DCL, 0, 2, VKD3DSIH_DCL}, /* Constant definitions */ - {VKD3D_SM1_OP_DEF, 1, 4, VKD3DSIH_DEF}, + {VKD3D_SM1_OP_DEF, 1, 1, VKD3DSIH_DEF}, {VKD3D_SM1_OP_DEFB, 1, 1, VKD3DSIH_DEFB}, - {VKD3D_SM1_OP_DEFI, 1, 4, VKD3DSIH_DEFI}, + {VKD3D_SM1_OP_DEFI, 1, 1, VKD3DSIH_DEFI}, /* Control flow */ {VKD3D_SM1_OP_REP, 0, 1, VKD3DSIH_REP, {2, 1}, {~0u, ~0u}}, {VKD3D_SM1_OP_ENDREP, 0, 0, VKD3DSIH_ENDREP, {2, 1}, {~0u, ~0u}}, -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/257
From: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl_constant_ops.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/libs/vkd3d-shader/hlsl_constant_ops.c b/libs/vkd3d-shader/hlsl_constant_ops.c index 570773cd..cde9afdb 100644 --- a/libs/vkd3d-shader/hlsl_constant_ops.c +++ b/libs/vkd3d-shader/hlsl_constant_ops.c @@ -80,7 +80,7 @@ static bool fold_cast(struct hlsl_ctx *ctx, struct hlsl_constant_value *dst, return false; } - for (k = 0; k < 4; ++k) + for (k = 0; k < dst_type->dimx; ++k) { switch (src->node.data_type->base_type) { @@ -160,7 +160,7 @@ static bool fold_neg(struct hlsl_ctx *ctx, struct hlsl_constant_value *dst, assert(type == src->node.data_type->base_type); - for (k = 0; k < 4; ++k) + for (k = 0; k < dst_type->dimx; ++k) { switch (type) { @@ -195,7 +195,7 @@ static bool fold_add(struct hlsl_ctx *ctx, struct hlsl_constant_value *dst, cons assert(type == src1->node.data_type->base_type); assert(type == src2->node.data_type->base_type); - for (k = 0; k < 4; ++k) + for (k = 0; k < dst_type->dimx; ++k) { switch (type) { @@ -380,7 +380,7 @@ static bool fold_equal(struct hlsl_ctx *ctx, struct hlsl_constant_value *dst, co assert(dst_type->base_type == HLSL_TYPE_BOOL); assert(src1->node.data_type->base_type == src2->node.data_type->base_type); - for (k = 0; k < 4; ++k) + for (k = 0; k < dst_type->dimx; ++k) { switch (src1->node.data_type->base_type) { @@ -416,7 +416,7 @@ static bool fold_gequal(struct hlsl_ctx *ctx, struct hlsl_constant_value *dst, c assert(dst_type->base_type == HLSL_TYPE_BOOL); assert(src1->node.data_type->base_type == src2->node.data_type->base_type); - for (k = 0; k < 4; ++k) + for (k = 0; k < dst_type->dimx; ++k) { switch (src1->node.data_type->base_type) { @@ -455,7 +455,7 @@ static bool fold_less(struct hlsl_ctx *ctx, struct hlsl_constant_value *dst, con assert(dst_type->base_type == HLSL_TYPE_BOOL); assert(src1->node.data_type->base_type == src2->node.data_type->base_type); - for (k = 0; k < 4; ++k) + for (k = 0; k < dst_type->dimx; ++k) { switch (src1->node.data_type->base_type) { @@ -614,7 +614,7 @@ static bool fold_mul(struct hlsl_ctx *ctx, struct hlsl_constant_value *dst, cons assert(type == src1->node.data_type->base_type); assert(type == src2->node.data_type->base_type); - for (k = 0; k < 4; ++k) + for (k = 0; k < dst_type->dimx; ++k) { switch (type) { @@ -648,7 +648,7 @@ static bool fold_nequal(struct hlsl_ctx *ctx, struct hlsl_constant_value *dst, c assert(dst_type->base_type == HLSL_TYPE_BOOL); assert(src1->node.data_type->base_type == src2->node.data_type->base_type); - for (k = 0; k < 4; ++k) + for (k = 0; k < dst_type->dimx; ++k) { switch (src1->node.data_type->base_type) { -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/257
I see that 2/2 removes conditional jump or move warnings such as: ``` ==259476== Conditional jump or move depends on uninitialised value(s) ==259476== at 0x48EF66B: fold_cast (hlsl_constant_ops.c:119) ==259476== by 0x48EF66B: hlsl_fold_constant_exprs (hlsl_constant_ops.c:717) ==259476== by 0x48EB806: hlsl_transform_ir (hlsl_codegen.c:570) ==259476== by 0x48EDEFF: hlsl_emit_bytecode (hlsl_codegen.c:4151) ==259476== by 0x48E652B: hlsl_compile_shader (hlsl.c:3385) ==259476== by 0x4917352: compile_hlsl (vkd3d_shader_main.c:1356) ==259476== by 0x4917352: vkd3d_shader_compile (vkd3d_shader_main.c:1429) ==259476== by 0x496B403: D3DCompile2 (vkd3d_utils_main.c:265) ==259476== by 0xB0686F631D5EDB4C: ??? ==259476== by 0x9B0000001FFEFFEF: ??? ``` However, I am curious on how 1/2 helps valgrind, since I don't see how a larger `ins->src_count` can cause problems in the `ins->handler_idx == VKD3DSIH_DEF` and `ins->handler_idx == VKD3DSIH_DEFI` branches. I am missing something? Additional parameters for valgrind, perhaps? Regardless of that, 1/2 seems to make the code more correct to me, so I am approving. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/257#note_37445
This merge request was approved by Francisco Casas. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/257
However, I am curious on how 1/2 helps valgrind, since I don't see how a larger `ins->src_count` can cause problems in the `ins->handler_idx == VKD3DSIH_DEF` and `ins->handler_idx == VKD3DSIH_DEFI` branches. I am missing something? Additional parameters for valgrind, perhaps?
No, it only shows up with some local patches for sm1 -> spirv. But I did use valgrind to find it, so, credit where it's due. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/257#note_37452
This merge request was approved by Giovanni Mascellani. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/257
This merge request was approved by Henri Verbeet. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/257
participants (5)
-
Francisco Casas (＠fcasas) -
Giovanni Mascellani (＠giomasce)
-
Henri Verbeet (＠hverbeet)
-
Zebediah Figura
-
Zebediah Figura (＠zfigura)