[PATCH 0/3] MR254: hlsl: Fix constant allocation for sm1.
From: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl_codegen.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index b980ed56..e265a36e 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -3359,8 +3359,6 @@ static void allocate_const_registers(struct hlsl_ctx *ctx, struct hlsl_ir_functi struct register_allocator allocator = {0}; struct hlsl_ir_var *var; - allocate_const_registers_recurse(ctx, &entry_func->body, &allocator); - LIST_FOR_EACH_ENTRY(var, &ctx->extern_vars, struct hlsl_ir_var, extern_entry) { if (var->is_uniform && var->last_read) @@ -3377,6 +3375,8 @@ static void allocate_const_registers(struct hlsl_ctx *ctx, struct hlsl_ir_functi } } + allocate_const_registers_recurse(ctx, &entry_func->body, &allocator); + vkd3d_free(allocator.allocations); } -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/254
From: Zebediah Figura <zfigura(a)codeweavers.com> That cannot happen with the current code. --- libs/vkd3d-shader/hlsl_codegen.c | 81 +++++++++++++++----------------- 1 file changed, 38 insertions(+), 43 deletions(-) diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index e265a36e..ce673d1d 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -3268,16 +3268,15 @@ static void allocate_const_registers_recurse(struct hlsl_ctx *ctx, { struct hlsl_ir_constant *constant = hlsl_ir_constant(instr); const struct hlsl_type *type = instr->data_type; - unsigned int x, y, i, writemask, end_reg; - unsigned int reg_size = type->reg_size[HLSL_REGSET_NUMERIC]; + unsigned int x, i, end_reg; constant->reg = allocate_numeric_registers_for_type(ctx, allocator, 1, UINT_MAX, type); TRACE("Allocated constant @%u to %s.\n", instr->index, debug_register('c', constant->reg, type)); if (!hlsl_array_reserve(ctx, (void **)&defs->values, &defs->size, - constant->reg.id + reg_size / 4, sizeof(*defs->values))) + constant->reg.id + 1, sizeof(*defs->values))) return; - end_reg = constant->reg.id + reg_size / 4; + end_reg = constant->reg.id + 1; if (end_reg > defs->count) { memset(&defs->values[defs->count], 0, sizeof(*defs->values) * (end_reg - defs->count)); @@ -3285,49 +3284,45 @@ static void allocate_const_registers_recurse(struct hlsl_ctx *ctx, } assert(type->class <= HLSL_CLASS_LAST_NUMERIC); + assert(type->dimy == 1); + assert(constant->reg.writemask); - if (!(writemask = constant->reg.writemask)) - writemask = (1u << type->dimx) - 1; - - for (y = 0; y < type->dimy; ++y) + for (x = 0, i = 0; x < 4; ++x) { - for (x = 0, i = 0; x < 4; ++x) + const union hlsl_constant_value_component *value; + float f; + + if (!(constant->reg.writemask & (1u << x))) + continue; + value = &constant->value.u[i++]; + + switch (type->base_type) { - const union hlsl_constant_value_component *value; - float f; - - if (!(writemask & (1u << x))) - continue; - value = &constant->value.u[i++]; - - switch (type->base_type) - { - case HLSL_TYPE_BOOL: - f = !!value->u; - break; - - case HLSL_TYPE_FLOAT: - case HLSL_TYPE_HALF: - f = value->f; - break; - - case HLSL_TYPE_INT: - f = value->i; - break; - - case HLSL_TYPE_UINT: - f = value->u; - break; - - case HLSL_TYPE_DOUBLE: - FIXME("Double constant.\n"); - return; - - default: - vkd3d_unreachable(); - } - defs->values[constant->reg.id + y].f[x] = f; + case HLSL_TYPE_BOOL: + f = !!value->u; + break; + + case HLSL_TYPE_FLOAT: + case HLSL_TYPE_HALF: + f = value->f; + break; + + case HLSL_TYPE_INT: + f = value->i; + break; + + case HLSL_TYPE_UINT: + f = value->u; + break; + + case HLSL_TYPE_DOUBLE: + FIXME("Double constant.\n"); + return; + + default: + vkd3d_unreachable(); } + defs->values[constant->reg.id].f[x] = f; } break; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/254
From: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/d3dbc.c | 5 ++-- libs/vkd3d-shader/hlsl.h | 6 ++++- libs/vkd3d-shader/hlsl_codegen.c | 40 +++++++++++++++++++++----------- 3 files changed, 35 insertions(+), 16 deletions(-) diff --git a/libs/vkd3d-shader/d3dbc.c b/libs/vkd3d-shader/d3dbc.c index d2a4666a..18141ae8 100644 --- a/libs/vkd3d-shader/d3dbc.c +++ b/libs/vkd3d-shader/d3dbc.c @@ -1809,12 +1809,13 @@ static void write_sm1_constant_defs(struct hlsl_ctx *ctx, struct vkd3d_bytecode_ for (i = 0; i < ctx->constant_defs.count; ++i) { + const struct hlsl_constant_register *constant_reg = &ctx->constant_defs.regs[i]; uint32_t token = D3DSIO_DEF; const struct sm1_dst_register reg = { .type = D3DSPR_CONST, .writemask = VKD3DSP_WRITEMASK_ALL, - .reg = i, + .reg = constant_reg->index, }; if (ctx->profile->major_version > 1) @@ -1823,7 +1824,7 @@ static void write_sm1_constant_defs(struct hlsl_ctx *ctx, struct vkd3d_bytecode_ write_sm1_dst_register(buffer, ®); for (x = 0; x < 4; ++x) - put_f32(buffer, ctx->constant_defs.values[i].f[x]); + put_f32(buffer, constant_reg->value.f[x]); } } diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index 28c7c691..8dedacc7 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -806,7 +806,11 @@ struct hlsl_ctx * Only used for SM1 profiles. */ struct hlsl_constant_defs { - struct hlsl_vec4 *values; + struct hlsl_constant_register + { + uint32_t index; + struct hlsl_vec4 value; + } *regs; size_t count, size; } constant_defs; /* Number of temp. registers required for the shader to run, i.e. the largest temp register diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index ce673d1d..8927e291 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -3254,10 +3254,33 @@ static void allocate_temp_registers_recurse(struct hlsl_ctx *ctx, } } +static void record_constant(struct hlsl_ctx *ctx, unsigned int component_index, float f) +{ + struct hlsl_constant_defs *defs = &ctx->constant_defs; + struct hlsl_constant_register *reg; + size_t i; + + for (i = 0; i < defs->count; ++i) + { + reg = &defs->regs[i]; + if (reg->index == (component_index / 4)) + { + reg->value.f[component_index % 4] = f; + return; + } + } + + if (!hlsl_array_reserve(ctx, (void **)&defs->regs, &defs->size, defs->count + 1, sizeof(*defs->regs))) + return; + reg = &defs->regs[defs->count++]; + memset(reg, 0, sizeof(*reg)); + reg->index = component_index / 4; + reg->value.f[component_index % 4] = f; +} + static void allocate_const_registers_recurse(struct hlsl_ctx *ctx, struct hlsl_block *block, struct register_allocator *allocator) { - struct hlsl_constant_defs *defs = &ctx->constant_defs; struct hlsl_ir_node *instr; LIST_FOR_EACH_ENTRY(instr, &block->instrs, struct hlsl_ir_node, entry) @@ -3268,21 +3291,11 @@ static void allocate_const_registers_recurse(struct hlsl_ctx *ctx, { struct hlsl_ir_constant *constant = hlsl_ir_constant(instr); const struct hlsl_type *type = instr->data_type; - unsigned int x, i, end_reg; + unsigned int x, i; constant->reg = allocate_numeric_registers_for_type(ctx, allocator, 1, UINT_MAX, type); TRACE("Allocated constant @%u to %s.\n", instr->index, debug_register('c', constant->reg, type)); - if (!hlsl_array_reserve(ctx, (void **)&defs->values, &defs->size, - constant->reg.id + 1, sizeof(*defs->values))) - return; - end_reg = constant->reg.id + 1; - if (end_reg > defs->count) - { - memset(&defs->values[defs->count], 0, sizeof(*defs->values) * (end_reg - defs->count)); - defs->count = end_reg; - } - assert(type->class <= HLSL_CLASS_LAST_NUMERIC); assert(type->dimy == 1); assert(constant->reg.writemask); @@ -3322,7 +3335,8 @@ static void allocate_const_registers_recurse(struct hlsl_ctx *ctx, default: vkd3d_unreachable(); } - defs->values[constant->reg.id].f[x] = f; + + record_constant(ctx, constant->reg.id * 4 + x, f); } break; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/254
The idea here is that `c` registers are used for both uniforms (when not defined) and constants (when defined)? Also, anonymous constants are moved after uniforms just because that's what native does, or is there a deeper reason? -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/254#note_37617
On Mon Jul 3 17:09:11 2023 +0000, Giovanni Mascellani wrote:
The idea here is that `c` registers are used for both uniforms (when not defined) and constants (when defined)? Also, anonymous constants are moved after uniforms just because that's what native does, or is there a deeper reason? Yes to both, that's how sm1 works and how native allocates constants.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/254#note_37639
This merge request was approved by Francisco Casas. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/254
By the way, we are not freeing `ctx->constant_defs.regs` in hlsl_ctx_cleanup(), but that could go in another MR. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/254#note_37683
This merge request was approved by Giovanni Mascellani. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/254
This merge request was approved by Henri Verbeet. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/254
On Mon Jul 3 17:09:11 2023 +0000, Zebediah Figura wrote:
Yes to both, that's how sm1 works and how native allocates constants. I suppose I should probably add that the motivation (and probably the reason that sm1 allocates uniforms first) is so that uniforms have a predictable register. We already have some tests that depend on that.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/254#note_37865
On Tue Jul 4 20:15:50 2023 +0000, Zebediah Figura wrote:
I suppose I should probably add that the motivation (and probably the reason that sm1 allocates uniforms first) is so that uniforms have a predictable register. We already have some tests that depend on that. Ah, yes, it makes sense!
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/254#note_37868
participants (5)
-
Francisco Casas (＠fcasas) -
Giovanni Mascellani (＠giomasce)
-
Henri Verbeet (＠hverbeet)
-
Zebediah Figura
-
Zebediah Figura (＠zfigura)