[PATCH 0/3] MR288: hlsl: Some fixes caught by Wine tests.
From: Zebediah Figura <zfigura(a)codeweavers.com> Instead of asserting. --- libs/vkd3d-shader/d3dbc.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/libs/vkd3d-shader/d3dbc.c b/libs/vkd3d-shader/d3dbc.c index 9a147e3d..34dc4bab 100644 --- a/libs/vkd3d-shader/d3dbc.c +++ b/libs/vkd3d-shader/d3dbc.c @@ -2034,7 +2034,12 @@ static void write_sm1_sampler_dcls(struct hlsl_ctx *ctx, struct vkd3d_bytecode_b if (var->objects_usage[HLSL_REGSET_SAMPLERS][i].used) { sampler_dim = var->objects_usage[HLSL_REGSET_SAMPLERS][i].sampler_dim; - assert(sampler_dim != HLSL_SAMPLER_DIM_GENERIC); + if (sampler_dim == HLSL_SAMPLER_DIM_GENERIC) + { + /* These can appear in sm4-style combined sample instructions. */ + hlsl_fixme(ctx, &var->loc, "Generic samplers need to be lowered."); + continue; + } reg_id = var->regs[HLSL_REGSET_SAMPLERS].id + i; write_sm1_sampler_dcl(ctx, buffer, reg_id, sampler_dim); -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/288
From: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/d3dbc.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/libs/vkd3d-shader/d3dbc.c b/libs/vkd3d-shader/d3dbc.c index 34dc4bab..48225234 100644 --- a/libs/vkd3d-shader/d3dbc.c +++ b/libs/vkd3d-shader/d3dbc.c @@ -2442,7 +2442,6 @@ static void write_sm1_instructions(struct hlsl_ctx *ctx, struct vkd3d_bytecode_b int hlsl_sm1_write(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *entry_func, struct vkd3d_shader_code *out) { struct vkd3d_bytecode_buffer buffer = {0}; - int ret; put_u32(&buffer, sm1_version(ctx->profile->type, ctx->profile->major_version, ctx->profile->minor_version)); @@ -2455,10 +2454,13 @@ int hlsl_sm1_write(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *entry_fun put_u32(&buffer, D3DSIO_END); - if (!(ret = buffer.status)) + if (buffer.status) + ctx->result = buffer.status; + + if (!ctx->result) { out->code = buffer.data; out->size = buffer.size; } - return ret; + return ctx->result; } -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/288
From: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/tpf.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/libs/vkd3d-shader/tpf.c b/libs/vkd3d-shader/tpf.c index 4be2f8b8..7969a67d 100644 --- a/libs/vkd3d-shader/tpf.c +++ b/libs/vkd3d-shader/tpf.c @@ -2627,7 +2627,8 @@ bool hlsl_sm4_usage_from_semantic(struct hlsl_ctx *ctx, const struct hlsl_semant return true; } -static void add_section(struct dxbc_writer *dxbc, uint32_t tag, struct vkd3d_bytecode_buffer *buffer) +static void add_section(struct hlsl_ctx *ctx, struct dxbc_writer *dxbc, + uint32_t tag, struct vkd3d_bytecode_buffer *buffer) { /* Native D3DDisassemble() expects at least the sizes of the ISGN and OSGN * sections to be aligned. Without this, the sections themselves will be @@ -2635,6 +2636,9 @@ static void add_section(struct dxbc_writer *dxbc, uint32_t tag, struct vkd3d_byt size_t size = bytecode_align(buffer); dxbc_writer_add_section(dxbc, tag, buffer->data, size); + + if (buffer->status < 0) + ctx->result = buffer->status; } static void write_sm4_signature(struct hlsl_ctx *ctx, struct dxbc_writer *dxbc, bool output) @@ -2742,7 +2746,7 @@ static void write_sm4_signature(struct hlsl_ctx *ctx, struct dxbc_writer *dxbc, set_u32(&buffer, count_position, i); - add_section(dxbc, output ? TAG_OSGN : TAG_ISGN, &buffer); + add_section(ctx, dxbc, output ? TAG_OSGN : TAG_ISGN, &buffer); } static D3D_SHADER_VARIABLE_CLASS sm4_class(const struct hlsl_type *type) @@ -3357,7 +3361,7 @@ static void write_sm4_rdef(struct hlsl_ctx *ctx, struct dxbc_writer *dxbc) creator_offset = put_string(&buffer, vkd3d_shader_get_version(NULL, NULL)); set_u32(&buffer, creator_position, creator_offset); - add_section(dxbc, TAG_RDEF, &buffer); + add_section(ctx, dxbc, TAG_RDEF, &buffer); sm4_free_extern_resources(extern_resources, extern_resources_count); } @@ -5322,7 +5326,7 @@ static void write_sm4_shdr(struct hlsl_ctx *ctx, set_u32(&buffer, token_count_position, bytecode_get_size(&buffer) / sizeof(uint32_t)); - add_section(dxbc, TAG_SHDR, &buffer); + add_section(ctx, dxbc, TAG_SHDR, &buffer); sm4_free_extern_resources(extern_resources, extern_resources_count); } -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/288
Francisco Casas (@fcasas) commented about libs/vkd3d-shader/d3dbc.c:
put_u32(&buffer, D3DSIO_END);
- if (!(ret = buffer.status)) + if (buffer.status) + ctx->result = buffer.status; + + if (!ctx->result) { out->code = buffer.data; out->size = buffer.size; } - return ret; + return ctx->result;
Wouldn't we be leaking `buffer.data` in case `ctx->result != 0`, but `buffer.status = 0` ? -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/288#note_40409
Wouldn't we be leaking `buffer.data` in case `ctx->result != 0`, but `buffer.status = 0` ?
Yes, in fact we're leaking it either way; if allocation fails the existing memory is not freed, so this was already a problem. I'll add a patch to fix it. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/288#note_40413
participants (3)
-
Francisco Casas (＠fcasas) -
Zebediah Figura
-
Zebediah Figura (＠zfigura)