[PATCH vkd3d 1/5] vkd3d-shader: Allocate temporary registers for variables.
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.c | 4 +- libs/vkd3d-shader/hlsl.h | 9 ++ libs/vkd3d-shader/hlsl_codegen.c | 184 +++++++++++++++++++++++++++++++ 3 files changed, 195 insertions(+), 2 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index 53b9db36..67ca2f2c 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -890,7 +890,7 @@ static void dump_deref(struct vkd3d_string_buffer *buffer, const struct hlsl_der } } -static const char *debug_writemask(DWORD writemask) +const char *debug_hlsl_writemask(unsigned int writemask) { static const char components[] = {'x', 'y', 'z', 'w'}; char string[5]; @@ -914,7 +914,7 @@ static void dump_ir_assignment(struct vkd3d_string_buffer *buffer, const struct vkd3d_string_buffer_printf(buffer, "= ("); dump_deref(buffer, &assign->lhs); if (assign->writemask != VKD3DSP_WRITEMASK_ALL) - vkd3d_string_buffer_printf(buffer, "%s", debug_writemask(assign->writemask)); + vkd3d_string_buffer_printf(buffer, "%s", debug_hlsl_writemask(assign->writemask)); vkd3d_string_buffer_printf(buffer, " "); dump_src(buffer, &assign->rhs); vkd3d_string_buffer_printf(buffer, ")"); diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index cf12f63f..fa7709ef 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -137,6 +137,13 @@ struct hlsl_struct_field unsigned int reg_offset; }; +struct hlsl_reg +{ + uint32_t id; + unsigned char writemask; + bool allocated; +}; + enum hlsl_ir_node_type { HLSL_IR_ASSIGNMENT = 0, @@ -208,6 +215,7 @@ struct hlsl_ir_var struct list scope_entry, param_entry; unsigned int first_write, last_read; + struct hlsl_reg reg; uint32_t is_input_varying : 1; uint32_t is_output_varying : 1; @@ -493,6 +501,7 @@ static inline void hlsl_src_remove(struct hlsl_src *src) } const char *debug_hlsl_type(const struct hlsl_type *type) DECLSPEC_HIDDEN; +const char *debug_hlsl_writemask(unsigned int writemask) DECLSPEC_HIDDEN; struct vkd3d_string_buffer *hlsl_type_to_string(struct vkd3d_string_buffer_cache *string_buffers, const struct hlsl_type *type) DECLSPEC_HIDDEN; diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index dbd591af..420e720f 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -612,6 +612,188 @@ static void compute_liveness(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl compute_liveness_recurse(entry_func->body, 0, 0); } +struct liveness +{ + size_t size; + struct + { + /* 0 if not live yet. */ + unsigned int last_read; + } *regs; +}; + +static unsigned char get_available_writemask(struct liveness *liveness, + unsigned int first_write, unsigned int index, unsigned int components) +{ + unsigned char i, writemask = 0, count = 0; + + for (i = 0; i < 4; ++i) + { + if (liveness->regs[index + i].last_read <= first_write) + { + writemask |= 1 << i; + if (++count == components) + return writemask; + } + } + + return 0; +} + +static bool resize_liveness(struct liveness *liveness, size_t new_count) +{ + size_t old_capacity = liveness->size; + + if (!vkd3d_array_reserve((void **)&liveness->regs, &liveness->size, new_count, sizeof(*liveness->regs))) + return false; + + if (liveness->size > old_capacity) + memset(liveness->regs + old_capacity, 0, (liveness->size - old_capacity) * sizeof(*liveness->regs)); + return true; +} + +static struct hlsl_reg allocate_register(struct liveness *liveness, + unsigned int first_write, unsigned int last_read, unsigned char components) +{ + struct hlsl_reg ret = {.allocated = true}; + unsigned char writemask, i; + unsigned int regnum; + + for (regnum = 0; regnum < liveness->size; regnum += 4) + { + if ((writemask = get_available_writemask(liveness, first_write, regnum, components))) + break; + } + if (regnum == liveness->size) + { + if (!resize_liveness(liveness, regnum + 4)) + return ret; + writemask = (1 << components) - 1; + } + for (i = 0; i < 4; ++i) + { + if (writemask & (1 << i)) + liveness->regs[regnum + i].last_read = last_read; + } + ret.id = regnum / 4; + ret.writemask = writemask; + return ret; +} + +static bool is_range_available(struct liveness *liveness, unsigned int first_write, + unsigned int index, unsigned int elements) +{ + unsigned int i; + + for (i = 0; i < elements; i += 4) + { + if (!get_available_writemask(liveness, first_write, index + i, 4)) + return false; + } + return true; +} + +/* "elements" is the total number of consecutive whole registers needed. */ +static struct hlsl_reg allocate_range(struct liveness *liveness, + unsigned int first_write, unsigned int last_read, unsigned int elements) +{ + const unsigned int components = elements * 4; + struct hlsl_reg ret = {.allocated = true}; + unsigned int i, regnum; + + for (regnum = 0; regnum < liveness->size; regnum += 4) + { + if (is_range_available(liveness, first_write, regnum, min(components, liveness->size - regnum))) + break; + } + if (!resize_liveness(liveness, regnum + components)) + return ret; + + for (i = 0; i < components; ++i) + liveness->regs[regnum + i].last_read = last_read; + ret.id = regnum / 4; + return ret; +} + +static const char *debug_register(char class, struct hlsl_reg reg, const struct hlsl_type *type) +{ + if (type->reg_size > 4) + return vkd3d_dbg_sprintf("%c%u-%c%u", class, reg.id, class, + reg.id + type->reg_size - 1); + return vkd3d_dbg_sprintf("%c%u%s", class, reg.id, debug_hlsl_writemask(reg.writemask)); +} + +static void allocate_variable_temp_register(struct hlsl_ir_var *var, struct liveness *liveness) +{ + if (var->is_input_varying || var->is_output_varying || var->is_uniform) + return; + + if (!var->reg.allocated && var->last_read) + { + if (var->data_type->reg_size > 1) + var->reg = allocate_range(liveness, var->first_write, + var->last_read, var->data_type->reg_size); + else + var->reg = allocate_register(liveness, var->first_write, + var->last_read, var->data_type->dimx); + TRACE("Allocated %s to %s (liveness %u-%u).\n", debug_register('r', var->reg, var->data_type), + var->name, var->first_write, var->last_read); + } +} + +static void allocate_temp_registers_recurse(struct list *instrs, struct liveness *liveness) +{ + struct hlsl_ir_node *instr; + + LIST_FOR_EACH_ENTRY(instr, instrs, struct hlsl_ir_node, entry) + { + switch (instr->type) + { + case HLSL_IR_ASSIGNMENT: + { + struct hlsl_ir_assignment *assignment = hlsl_ir_assignment(instr); + allocate_variable_temp_register(assignment->lhs.var, liveness); + break; + } + + case HLSL_IR_IF: + { + struct hlsl_ir_if *iff = hlsl_ir_if(instr); + allocate_temp_registers_recurse(&iff->then_instrs, liveness); + allocate_temp_registers_recurse(&iff->else_instrs, liveness); + break; + } + + case HLSL_IR_LOAD: + { + struct hlsl_ir_load *load = hlsl_ir_load(instr); + allocate_variable_temp_register(load->src.var, liveness); + break; + } + + case HLSL_IR_LOOP: + { + struct hlsl_ir_loop *loop = hlsl_ir_loop(instr); + allocate_temp_registers_recurse(&loop->body, liveness); + break; + } + + default: + break; + } + } +} + +/* Simple greedy temporary register allocation pass that just assigns a unique + * index to all (simultaneously live) variables or intermediate values. Agnostic + * as to how many registers are actually available for the current backend, and + * does not handle constants. */ +static void allocate_temp_registers(struct hlsl_ir_function_decl *entry_func) +{ + struct liveness liveness = {0}; + allocate_temp_registers_recurse(entry_func->body, &liveness); +} + int hlsl_emit_dxbc(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *entry_func) { struct hlsl_ir_var *var; @@ -649,6 +831,8 @@ int hlsl_emit_dxbc(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *entry_fun if (TRACE_ON()) rb_for_each_entry(&ctx->functions, dump_function, NULL); + allocate_temp_registers(entry_func); + if (ctx->failed) return VKD3D_ERROR_INVALID_SHADER; return VKD3D_ERROR_NOT_IMPLEMENTED; -- 2.31.0
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.h | 1 + libs/vkd3d-shader/hlsl_codegen.c | 13 +++++++++++++ 2 files changed, 14 insertions(+) diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index fa7709ef..664137ce 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -171,6 +171,7 @@ struct hlsl_ir_node * true even for loops, since currently we can't have a reference to a * value generated in an earlier iteration of the loop. */ unsigned int index, last_read; + struct hlsl_reg reg; }; struct hlsl_src diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 420e720f..42e8f86a 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -747,6 +747,19 @@ static void allocate_temp_registers_recurse(struct list *instrs, struct liveness LIST_FOR_EACH_ENTRY(instr, instrs, struct hlsl_ir_node, entry) { + if (!instr->reg.allocated && instr->last_read) + { + if (instr->data_type->reg_size > 1) + instr->reg = allocate_range(liveness, instr->index, + instr->last_read, instr->data_type->reg_size); + else + instr->reg = allocate_register(liveness, instr->index, + instr->last_read, instr->data_type->dimx); + TRACE("Allocated %s to anonymous expression @%u (liveness %u-%u).\n", + debug_register('r', instr->reg, instr->data_type), + instr->index, instr->index, instr->last_read); + } + switch (instr->type) { case HLSL_IR_ASSIGNMENT: -- 2.31.0
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.c | 9 +++++++-- libs/vkd3d-shader/hlsl.h | 4 ++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index 67ca2f2c..c1f03978 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -1556,10 +1556,15 @@ static void declare_predefined_types(struct hlsl_ctx *ctx) } } -static bool hlsl_ctx_init(struct hlsl_ctx *ctx, struct vkd3d_shader_message_context *message_context) +static bool hlsl_ctx_init(struct hlsl_ctx *ctx, const struct hlsl_profile_info *profile, + struct vkd3d_shader_message_context *message_context) { memset(ctx, 0, sizeof(*ctx)); + ctx->shader_type = profile->type; + ctx->major_version = profile->sm_major; + ctx->minor_version = profile->sm_minor; + ctx->message_context = message_context; if (!(ctx->source_files = vkd3d_malloc(sizeof(*ctx->source_files)))) @@ -1642,7 +1647,7 @@ int hlsl_compile_shader(const struct vkd3d_shader_code *hlsl, const struct vkd3d vkd3d_shader_dump_shader(profile->type, &compile_info->source); - if (!hlsl_ctx_init(&ctx, message_context)) + if (!hlsl_ctx_init(&ctx, profile, message_context)) return VKD3D_ERROR_OUT_OF_MEMORY; if (hlsl_lexer_compile(&ctx, hlsl) == 2) diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index 664137ce..475119d1 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -380,6 +380,7 @@ struct hlsl_ir_constant double d[4]; bool b[4]; } value; + struct hlsl_reg reg; }; struct hlsl_scope @@ -392,6 +393,9 @@ struct hlsl_scope struct hlsl_ctx { + enum vkd3d_shader_type shader_type; + unsigned int major_version, minor_version; + const char **source_files; unsigned int source_files_count; struct vkd3d_shader_location location; -- 2.31.0
On Wed, Mar 31, 2021 at 12:04 AM Zebediah Figura <zfigura(a)codeweavers.com> wrote:
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.c | 9 +++++++-- libs/vkd3d-shader/hlsl.h | 4 ++++ 2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index 67ca2f2c..c1f03978 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -1556,10 +1556,15 @@ static void declare_predefined_types(struct hlsl_ctx *ctx) } }
-static bool hlsl_ctx_init(struct hlsl_ctx *ctx, struct vkd3d_shader_message_context *message_context) +static bool hlsl_ctx_init(struct hlsl_ctx *ctx, const struct hlsl_profile_info *profile, + struct vkd3d_shader_message_context *message_context) { memset(ctx, 0, sizeof(*ctx));
+ ctx->shader_type = profile->type; + ctx->major_version = profile->sm_major; + ctx->minor_version = profile->sm_minor; + ctx->message_context = message_context;
if (!(ctx->source_files = vkd3d_malloc(sizeof(*ctx->source_files)))) @@ -1642,7 +1647,7 @@ int hlsl_compile_shader(const struct vkd3d_shader_code *hlsl, const struct vkd3d
vkd3d_shader_dump_shader(profile->type, &compile_info->source);
- if (!hlsl_ctx_init(&ctx, message_context)) + if (!hlsl_ctx_init(&ctx, profile, message_context)) return VKD3D_ERROR_OUT_OF_MEMORY;
if (hlsl_lexer_compile(&ctx, hlsl) == 2) diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index 664137ce..475119d1 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -380,6 +380,7 @@ struct hlsl_ir_constant double d[4]; bool b[4]; } value; + struct hlsl_reg reg; };
struct hlsl_scope @@ -392,6 +393,9 @@ struct hlsl_scope
struct hlsl_ctx { + enum vkd3d_shader_type shader_type; + unsigned int major_version, minor_version; + const char **source_files; unsigned int source_files_count; struct vkd3d_shader_location location;
Otherwise we could store a pointer to the hlsl_profile_info structure. Not sure if that would make things more complicated down the line.
On 4/2/21 3:05 PM, Matteo Bruni wrote:
On Wed, Mar 31, 2021 at 12:04 AM Zebediah Figura <zfigura(a)codeweavers.com> wrote:
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.c | 9 +++++++-- libs/vkd3d-shader/hlsl.h | 4 ++++ 2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index 67ca2f2c..c1f03978 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -1556,10 +1556,15 @@ static void declare_predefined_types(struct hlsl_ctx *ctx) } }
-static bool hlsl_ctx_init(struct hlsl_ctx *ctx, struct vkd3d_shader_message_context *message_context) +static bool hlsl_ctx_init(struct hlsl_ctx *ctx, const struct hlsl_profile_info *profile, + struct vkd3d_shader_message_context *message_context) { memset(ctx, 0, sizeof(*ctx));
+ ctx->shader_type = profile->type; + ctx->major_version = profile->sm_major; + ctx->minor_version = profile->sm_minor; + ctx->message_context = message_context;
if (!(ctx->source_files = vkd3d_malloc(sizeof(*ctx->source_files)))) @@ -1642,7 +1647,7 @@ int hlsl_compile_shader(const struct vkd3d_shader_code *hlsl, const struct vkd3d
vkd3d_shader_dump_shader(profile->type, &compile_info->source);
- if (!hlsl_ctx_init(&ctx, message_context)) + if (!hlsl_ctx_init(&ctx, profile, message_context)) return VKD3D_ERROR_OUT_OF_MEMORY;
if (hlsl_lexer_compile(&ctx, hlsl) == 2) diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index 664137ce..475119d1 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -380,6 +380,7 @@ struct hlsl_ir_constant double d[4]; bool b[4]; } value; + struct hlsl_reg reg; };
struct hlsl_scope @@ -392,6 +393,9 @@ struct hlsl_scope
struct hlsl_ctx { + enum vkd3d_shader_type shader_type; + unsigned int major_version, minor_version; + const char **source_files; unsigned int source_files_count; struct vkd3d_shader_location location;
Otherwise we could store a pointer to the hlsl_profile_info structure. Not sure if that would make things more complicated down the line.
I don't think it makes a particular difference, no; that seems like a good idea.
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl_codegen.c | 51 ++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 42e8f86a..505a77e7 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -797,6 +797,55 @@ static void allocate_temp_registers_recurse(struct list *instrs, struct liveness } } +static void allocate_const_registers_recurse(struct list *instrs, struct liveness *liveness) +{ + struct hlsl_ir_node *instr; + + LIST_FOR_EACH_ENTRY(instr, instrs, struct hlsl_ir_node, entry) + { + switch (instr->type) + { + case HLSL_IR_CONSTANT: + { + struct hlsl_ir_constant *constant = hlsl_ir_constant(instr); + + if (instr->data_type->reg_size > 1) + constant->reg = allocate_range(liveness, 1, INT_MAX, instr->data_type->reg_size); + else + constant->reg = allocate_register(liveness, 1, INT_MAX, instr->data_type->dimx); + TRACE("Allocated %s to constant @%u.\n", debug_register('c', constant->reg, instr->data_type), + instr->index); + break; + } + + case HLSL_IR_IF: + { + struct hlsl_ir_if *iff = hlsl_ir_if(instr); + allocate_const_registers_recurse(&iff->then_instrs, liveness); + allocate_const_registers_recurse(&iff->else_instrs, liveness); + break; + } + + case HLSL_IR_LOOP: + { + struct hlsl_ir_loop *loop = hlsl_ir_loop(instr); + allocate_const_registers_recurse(&loop->body, liveness); + break; + } + + default: + break; + } + } +} + +static void allocate_const_registers(struct hlsl_ir_function_decl *entry_func) +{ + struct liveness liveness = {0}; + + allocate_const_registers_recurse(entry_func->body, &liveness); +} + /* Simple greedy temporary register allocation pass that just assigns a unique * index to all (simultaneously live) variables or intermediate values. Agnostic * as to how many registers are actually available for the current backend, and @@ -845,6 +894,8 @@ int hlsl_emit_dxbc(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *entry_fun rb_for_each_entry(&ctx->functions, dump_function, NULL); allocate_temp_registers(entry_func); + if (ctx->major_version < 4) + allocate_const_registers(entry_func); if (ctx->failed) return VKD3D_ERROR_INVALID_SHADER; -- 2.31.0
On Wed, Mar 31, 2021 at 12:04 AM Zebediah Figura <zfigura(a)codeweavers.com> wrote:
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl_codegen.c | 51 ++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+)
diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 42e8f86a..505a77e7 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -797,6 +797,55 @@ static void allocate_temp_registers_recurse(struct list *instrs, struct liveness } }
+static void allocate_const_registers_recurse(struct list *instrs, struct liveness *liveness) +{ + struct hlsl_ir_node *instr; + + LIST_FOR_EACH_ENTRY(instr, instrs, struct hlsl_ir_node, entry) + { + switch (instr->type) + { + case HLSL_IR_CONSTANT: + { + struct hlsl_ir_constant *constant = hlsl_ir_constant(instr); + + if (instr->data_type->reg_size > 1) + constant->reg = allocate_range(liveness, 1, INT_MAX, instr->data_type->reg_size); + else + constant->reg = allocate_register(liveness, 1, INT_MAX, instr->data_type->dimx);
Not that it makes a difference in practice, but shouldn't this be UINT_MAX in theory?
On 4/2/21 3:05 PM, Matteo Bruni wrote:
On Wed, Mar 31, 2021 at 12:04 AM Zebediah Figura <zfigura(a)codeweavers.com> wrote:
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl_codegen.c | 51 ++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+)
diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 42e8f86a..505a77e7 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -797,6 +797,55 @@ static void allocate_temp_registers_recurse(struct list *instrs, struct liveness } }
+static void allocate_const_registers_recurse(struct list *instrs, struct liveness *liveness) +{ + struct hlsl_ir_node *instr; + + LIST_FOR_EACH_ENTRY(instr, instrs, struct hlsl_ir_node, entry) + { + switch (instr->type) + { + case HLSL_IR_CONSTANT: + { + struct hlsl_ir_constant *constant = hlsl_ir_constant(instr); + + if (instr->data_type->reg_size > 1) + constant->reg = allocate_range(liveness, 1, INT_MAX, instr->data_type->reg_size); + else + constant->reg = allocate_register(liveness, 1, INT_MAX, instr->data_type->dimx);
Not that it makes a difference in practice, but shouldn't this be UINT_MAX in theory?
Yep, that's probably just an error...
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl_codegen.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 505a77e7..611cd7d7 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -839,9 +839,25 @@ static void allocate_const_registers_recurse(struct list *instrs, struct livenes } } -static void allocate_const_registers(struct hlsl_ir_function_decl *entry_func) +static void allocate_const_registers(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *entry_func) { struct liveness liveness = {0}; + struct hlsl_ir_var *var; + + LIST_FOR_EACH_ENTRY(var, &ctx->globals->vars, struct hlsl_ir_var, scope_entry) + { + if (var->is_uniform && var->last_read) + { + if (var->data_type->reg_size > 1) + var->reg = allocate_range(&liveness, 1, INT_MAX, var->data_type->reg_size); + else + { + var->reg = allocate_register(&liveness, 1, INT_MAX, 4); + var->reg.writemask = (1 << var->data_type->dimx) - 1; + } + TRACE("Allocated %s to %s.\n", debug_register('c', var->reg, var->data_type), var->name); + } + } allocate_const_registers_recurse(entry_func->body, &liveness); } @@ -895,7 +911,7 @@ int hlsl_emit_dxbc(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *entry_fun allocate_temp_registers(entry_func); if (ctx->major_version < 4) - allocate_const_registers(entry_func); + allocate_const_registers(ctx, entry_func); if (ctx->failed) return VKD3D_ERROR_INVALID_SHADER; -- 2.31.0
On Wed, Mar 31, 2021 at 12:04 AM Zebediah Figura <zfigura(a)codeweavers.com> wrote:
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.c | 4 +- libs/vkd3d-shader/hlsl.h | 9 ++ libs/vkd3d-shader/hlsl_codegen.c | 184 +++++++++++++++++++++++++++++++ 3 files changed, 195 insertions(+), 2 deletions(-)
This code feels strangely familiar ;) I have some nitpicks, as usual.
diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index dbd591af..420e720f 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -612,6 +612,188 @@ static void compute_liveness(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl compute_liveness_recurse(entry_func->body, 0, 0); }
+struct liveness +{ + size_t size; + struct + { + /* 0 if not live yet. */ + unsigned int last_read; + } *regs; +}; + +static unsigned char get_available_writemask(struct liveness *liveness, + unsigned int first_write, unsigned int index, unsigned int components) +{ + unsigned char i, writemask = 0, count = 0; + + for (i = 0; i < 4; ++i) + { + if (liveness->regs[index + i].last_read <= first_write) + { + writemask |= 1 << i; + if (++count == components) + return writemask; + } + } + + return 0; +} + +static bool resize_liveness(struct liveness *liveness, size_t new_count) +{ + size_t old_capacity = liveness->size; + + if (!vkd3d_array_reserve((void **)&liveness->regs, &liveness->size, new_count, sizeof(*liveness->regs))) + return false; + + if (liveness->size > old_capacity) + memset(liveness->regs + old_capacity, 0, (liveness->size - old_capacity) * sizeof(*liveness->regs)); + return true; +} + +static struct hlsl_reg allocate_register(struct liveness *liveness, + unsigned int first_write, unsigned int last_read, unsigned char components)
This is the only instance of "components" (which actually should be "component_count") that isn't unsigned int. I assume there was some back and forth while writing the patch.
+{ + struct hlsl_reg ret = {.allocated = true}; + unsigned char writemask, i; + unsigned int regnum;
No reason to use an unsigned char for writemask or i. "regnum" should probably be "index" (or "base_index", or "base", or something).
+ + for (regnum = 0; regnum < liveness->size; regnum += 4) + { + if ((writemask = get_available_writemask(liveness, first_write, regnum, components))) + break; + } + if (regnum == liveness->size) + { + if (!resize_liveness(liveness, regnum + 4)) + return ret; + writemask = (1 << components) - 1;
1u (also elsewhere).
+ } + for (i = 0; i < 4; ++i) + { + if (writemask & (1 << i)) + liveness->regs[regnum + i].last_read = last_read; + } + ret.id = regnum / 4; + ret.writemask = writemask; + return ret; +} + +static bool is_range_available(struct liveness *liveness, unsigned int first_write, + unsigned int index, unsigned int elements) +{ + unsigned int i; + + for (i = 0; i < elements; i += 4) + { + if (!get_available_writemask(liveness, first_write, index + i, 4)) + return false; + } + return true; +} + +/* "elements" is the total number of consecutive whole registers needed. */
Let's call it register_count or element_count. Or just count.
+static void allocate_variable_temp_register(struct hlsl_ir_var *var, struct liveness *liveness) +{ + if (var->is_input_varying || var->is_output_varying || var->is_uniform) + return; + + if (!var->reg.allocated && var->last_read) + { + if (var->data_type->reg_size > 1) + var->reg = allocate_range(liveness, var->first_write, + var->last_read, var->data_type->reg_size); + else + var->reg = allocate_register(liveness, var->first_write, + var->last_read, var->data_type->dimx); + TRACE("Allocated %s to %s (liveness %u-%u).\n", debug_register('r', var->reg, var->data_type), + var->name, var->first_write, var->last_read); + }
I guess both work, but it sounds better to me as "Allocated <variable name> to <register>".
+} + +static void allocate_temp_registers_recurse(struct list *instrs, struct liveness *liveness) +{ + struct hlsl_ir_node *instr; + + LIST_FOR_EACH_ENTRY(instr, instrs, struct hlsl_ir_node, entry) + { + switch (instr->type) + { + case HLSL_IR_ASSIGNMENT:
Entirely an aside: ASSIGNMENT looks pretty ugly now that we have LOAD for the symmetric operation and that the IR has been reworked into a somewhat proper load / store architecture. Up to you when (or if) to do a compiler-wide rename, just know that every time I read HLSL_IR_ASSIGNMENT I feel a tiiiiiiny bit of pain :P
+ { + struct hlsl_ir_assignment *assignment = hlsl_ir_assignment(instr); + allocate_variable_temp_register(assignment->lhs.var, liveness); + break; + } + + case HLSL_IR_IF: + { + struct hlsl_ir_if *iff = hlsl_ir_if(instr); + allocate_temp_registers_recurse(&iff->then_instrs, liveness); + allocate_temp_registers_recurse(&iff->else_instrs, liveness); + break; + } + + case HLSL_IR_LOAD: + { + struct hlsl_ir_load *load = hlsl_ir_load(instr); + allocate_variable_temp_register(load->src.var, liveness); + break; + }
This one is required because of input arguments and uniforms, I gather. Maybe a comment would be useful? I am not sure, honestly.
On 4/2/21 3:04 PM, Matteo Bruni wrote:
On Wed, Mar 31, 2021 at 12:04 AM Zebediah Figura <zfigura(a)codeweavers.com> wrote:
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.c | 4 +- libs/vkd3d-shader/hlsl.h | 9 ++ libs/vkd3d-shader/hlsl_codegen.c | 184 +++++++++++++++++++++++++++++++ 3 files changed, 195 insertions(+), 2 deletions(-)
This code feels strangely familiar ;)
I have some nitpicks, as usual.
diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index dbd591af..420e720f 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -612,6 +612,188 @@ static void compute_liveness(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl compute_liveness_recurse(entry_func->body, 0, 0); }
+struct liveness +{ + size_t size; + struct + { + /* 0 if not live yet. */ + unsigned int last_read; + } *regs; +}; + +static unsigned char get_available_writemask(struct liveness *liveness, + unsigned int first_write, unsigned int index, unsigned int components) +{ + unsigned char i, writemask = 0, count = 0; + + for (i = 0; i < 4; ++i) + { + if (liveness->regs[index + i].last_read <= first_write) + { + writemask |= 1 << i; + if (++count == components) + return writemask; + } + } + + return 0; +} + +static bool resize_liveness(struct liveness *liveness, size_t new_count) +{ + size_t old_capacity = liveness->size; + + if (!vkd3d_array_reserve((void **)&liveness->regs, &liveness->size, new_count, sizeof(*liveness->regs))) + return false; + + if (liveness->size > old_capacity) + memset(liveness->regs + old_capacity, 0, (liveness->size - old_capacity) * sizeof(*liveness->regs)); + return true; +} + +static struct hlsl_reg allocate_register(struct liveness *liveness, + unsigned int first_write, unsigned int last_read, unsigned char components)
This is the only instance of "components" (which actually should be "component_count") that isn't unsigned int. I assume there was some back and forth while writing the patch.
Yep... (Can you tell this patch is very old?)
+{ + struct hlsl_reg ret = {.allocated = true}; + unsigned char writemask, i; + unsigned int regnum;
No reason to use an unsigned char for writemask or i. "regnum" should probably be "index" (or "base_index", or "base", or something).
+ + for (regnum = 0; regnum < liveness->size; regnum += 4) + { + if ((writemask = get_available_writemask(liveness, first_write, regnum, components))) + break; + } + if (regnum == liveness->size) + { + if (!resize_liveness(liveness, regnum + 4)) + return ret; + writemask = (1 << components) - 1;
1u (also elsewhere).
+ } + for (i = 0; i < 4; ++i) + { + if (writemask & (1 << i)) + liveness->regs[regnum + i].last_read = last_read; + } + ret.id = regnum / 4; + ret.writemask = writemask; + return ret; +} + +static bool is_range_available(struct liveness *liveness, unsigned int first_write, + unsigned int index, unsigned int elements) +{ + unsigned int i; + + for (i = 0; i < elements; i += 4) + { + if (!get_available_writemask(liveness, first_write, index + i, 4)) + return false; + } + return true; +} + +/* "elements" is the total number of consecutive whole registers needed. */
Let's call it register_count or element_count. Or just count.
+static void allocate_variable_temp_register(struct hlsl_ir_var *var, struct liveness *liveness) +{ + if (var->is_input_varying || var->is_output_varying || var->is_uniform) + return; + + if (!var->reg.allocated && var->last_read) + { + if (var->data_type->reg_size > 1) + var->reg = allocate_range(liveness, var->first_write, + var->last_read, var->data_type->reg_size); + else + var->reg = allocate_register(liveness, var->first_write, + var->last_read, var->data_type->dimx); + TRACE("Allocated %s to %s (liveness %u-%u).\n", debug_register('r', var->reg, var->data_type), + var->name, var->first_write, var->last_read); + }
I guess both work, but it sounds better to me as "Allocated <variable name> to <register>".
+} + +static void allocate_temp_registers_recurse(struct list *instrs, struct liveness *liveness) +{ + struct hlsl_ir_node *instr; + + LIST_FOR_EACH_ENTRY(instr, instrs, struct hlsl_ir_node, entry) + { + switch (instr->type) + { + case HLSL_IR_ASSIGNMENT:
Entirely an aside: ASSIGNMENT looks pretty ugly now that we have LOAD for the symmetric operation and that the IR has been reworked into a somewhat proper load / store architecture. Up to you when (or if) to do a compiler-wide rename, just know that every time I read HLSL_IR_ASSIGNMENT I feel a tiiiiiiny bit of pain :P
Yeah, that was on my list to do 'eventually', but now is a good time :D
+ { + struct hlsl_ir_assignment *assignment = hlsl_ir_assignment(instr); + allocate_variable_temp_register(assignment->lhs.var, liveness); + break; + } + + case HLSL_IR_IF: + { + struct hlsl_ir_if *iff = hlsl_ir_if(instr); + allocate_temp_registers_recurse(&iff->then_instrs, liveness); + allocate_temp_registers_recurse(&iff->else_instrs, liveness); + break; + } + + case HLSL_IR_LOAD: + { + struct hlsl_ir_load *load = hlsl_ir_load(instr); + allocate_variable_temp_register(load->src.var, liveness); + break; + }
This one is required because of input arguments and uniforms, I gather. Maybe a comment would be useful? I am not sure, honestly.
Assuming I understand correctly, I don't think it's necessary per se (we may have to assign them to a temp reg for valid DXBC, but 3d22df25fe1 and friends already give us a temp variable for that purpose). A more salient reason is that the variable itself can be modified in the meantime, which in practice happens with postincrement.
participants (3)
-
Matteo Bruni -
Zebediah Figura -
Zebediah Figura (she/her)