[PATCH vkd3d 1/5] vkd3d-shader: Allocate temporary registers for variables.

[Zebediah Figura]

Signed-off-by: Zebediah Figura 
---
 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;

[PATCH vkd3d 2/5] vkd3d-shader: Allocate temporary registers for anonymous expressions.

[Zebediah Figura]

Signed-off-by: Zebediah Figura 
---
 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:

[PATCH vkd3d 3/5] vkd3d-shader: Store the shader type and version in the hlsl_ctx structure.

[Zebediah Figura]

Signed-off-by: Zebediah Figura 
---
 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;

[PATCH vkd3d 3/5] vkd3d-shader: Store the shader type and version in the hlsl_ctx structure.

[Matteo Bruni]

On Wed, Mar 31, 2021 at 12:04 AM Zebediah Figura
 wrote:
> Signed-off-by: Zebediah Figura
>
> 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.

[PATCH vkd3d 3/5] vkd3d-shader: Store the shader type and version in the hlsl_ctx structure.

[Zebediah Figura (she/her)]

On 4/2/21 3:05 PM, Matteo Bruni wrote:
> On Wed, Mar 31, 2021 at 12:04 AM Zebediah Figura
>  wrote:
> > Signed-off-by: Zebediah Figura
> >
> > 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.

[PATCH vkd3d 4/5] vkd3d-shader: Allocate constant registers for anonymous constants.

[Zebediah Figura]

Signed-off-by: Zebediah Figura 
---
 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;

[PATCH vkd3d 4/5] vkd3d-shader: Allocate constant registers for anonymous constants.

[Matteo Bruni]

On Wed, Mar 31, 2021 at 12:04 AM Zebediah Figura
 wrote:
> Signed-off-by: Zebediah Figura
>
> 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?

[PATCH vkd3d 4/5] vkd3d-shader: Allocate constant registers for anonymous constants.

[Zebediah Figura (she/her)]

On 4/2/21 3:05 PM, Matteo Bruni wrote:
> On Wed, Mar 31, 2021 at 12:04 AM Zebediah Figura
>  wrote:
> > Signed-off-by: Zebediah Figura
> >
> > 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...

[PATCH vkd3d 5/5] vkd3d-shader: Allocate constant registers for uniforms.

[Zebediah Figura]

Signed-off-by: Zebediah Figura 
---
 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;

[Matteo Bruni]

On Wed, Mar 31, 2021 at 12:04 AM Zebediah Figura
 wrote:
> Signed-off-by: Zebediah Figura
>
> 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.

[Zebediah Figura (she/her)]

On 4/2/21 3:04 PM, Matteo Bruni wrote:
> On Wed, Mar 31, 2021 at 12:04 AM Zebediah Figura
>  wrote:
> > Signed-off-by: Zebediah Figura
> >
> > 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.