[PATCH v2 vkd3d] vkd3d-shader/hlsl: Allocate samplers.

[Matteo Bruni]

From: Zebediah Figura

Signed-off-by: Matteo Bruni 
---
Now it should actually apply on top of 219117.

libs/vkd3d-shader/hlsl_codegen.c | 59 ++++++++++++++++++++++++--------
 1 file changed, 44 insertions(+), 15 deletions(-)

diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c
index 0bf18c2b..7c0b0f95 100644
--- a/libs/vkd3d-shader/hlsl_codegen.c
+++ b/libs/vkd3d-shader/hlsl_codegen.c
@@ -1202,60 +1202,88 @@ static void allocate_buffers(struct hlsl_ctx *ctx)
     }
 }

-static const struct hlsl_ir_var *get_reserved_texture(struct hlsl_ctx *ctx, uint32_t index)
+static const struct hlsl_ir_var *get_reserved_object(struct hlsl_ctx *ctx, char type, uint32_t index)
 {
     const struct hlsl_ir_var *var;

LIST_FOR_EACH_ENTRY(var, &ctx->extern_vars, const struct hlsl_ir_var, extern_entry)
     {
-        if (var->last_read && var->reg_reservation.type == 't' && var->reg_reservation.index == index)
+        if (var->last_read && var->reg_reservation.type == type && var->reg_reservation.index == index)
             return var;
     }
     return NULL;
 }

-static void allocate_textures(struct hlsl_ctx *ctx)
+static const struct object_type_info
 {
+    enum hlsl_base_type type;
+    char *name;
+    char reg_name;
+}
+object_types[] =
+{
+    { HLSL_TYPE_SAMPLER, "sampler", 's' },
+    { HLSL_TYPE_TEXTURE, "texture", 't' },
+};
+
+static const struct object_type_info *get_object_type_info(enum hlsl_base_type type)
+{
+    unsigned int i;
+
+    for (i = 0; i < ARRAY_SIZE(object_types); ++i)
+        if (type == object_types[i].type)
+            return &object_types[i];
+
+    WARN("No type info for object type %u.\n", type);
+    return NULL;
+}
+
+static void allocate_objects(struct hlsl_ctx *ctx, enum hlsl_base_type type)
+{
+    const struct object_type_info *type_info = get_object_type_info(type);
     struct hlsl_ir_var *var;
     uint32_t index = 0;

LIST_FOR_EACH_ENTRY(var, &ctx->extern_vars, struct hlsl_ir_var, extern_entry)
     {
         if (!var->last_read || var->data_type->type != HLSL_CLASS_OBJECT
-                || var->data_type->base_type != HLSL_TYPE_TEXTURE)
+                || var->data_type->base_type != type)
             continue;

-        if (var->reg_reservation.type == 't')
+        if (var->reg_reservation.type == type_info->reg_name)
         {
-            const struct hlsl_ir_var *reserved_texture = get_reserved_texture(ctx, var->reg_reservation.index);
+            const struct hlsl_ir_var *reserved_object = get_reserved_object(ctx, type_info->reg_name,
+                    var->reg_reservation.index);

-            if (reserved_texture && reserved_texture != var)
+            if (reserved_object && reserved_object != var)
             {
                 hlsl_error(ctx, var->loc, VKD3D_SHADER_ERROR_HLSL_OVERLAPPING_RESERVATIONS,
-                        "Multiple textures bound to t%u.", var->reg_reservation.index);
-                hlsl_note(ctx, reserved_texture->loc, VKD3D_SHADER_LOG_ERROR,
-                        "Texture '%s' is already bound to t%u.", reserved_texture->name,
+                        "Multiple %ss bound to %c%u.", type_info->name, type_info->reg_name,
                         var->reg_reservation.index);
+                hlsl_note(ctx, reserved_object->loc, VKD3D_SHADER_LOG_ERROR,
+                        "Object '%s' is already bound to %c%u.", reserved_object->name,
+                        type_info->reg_name, var->reg_reservation.index);
             }

var->reg.id = var->reg_reservation.index;
             var->reg.allocated = true;
-            TRACE("Allocated reserved %s to t%u.\n", var->name, var->reg_reservation.index);
+            TRACE("Allocated reserved %s to %c%u.\n", var->name, type_info->reg_name, var->reg_reservation.index);
         }
         else if (!var->reg_reservation.type)
         {
-            while (get_reserved_texture(ctx, index))
+            while (get_reserved_object(ctx, type_info->reg_name, index))
                 ++index;

var->reg.id = index;
             var->reg.allocated = true;
-            TRACE("Allocated %s to t%u.\n", var->name, index);
+            TRACE("Allocated %s to %c%u.\n", var->name, type_info->reg_name, index);
             ++index;
         }
         else
         {
             hlsl_error(ctx, var->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_RESERVATION,
-                    "Textures must be bound to register type 't'.");
+                    "Object of type '%s' must be bound to register type '%c'.",
+                    type_info->name, type_info->reg_name);
         }
     }
 }
@@ -1376,9 +1404,10 @@ int hlsl_emit_dxbc(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *entry_fun
     else
     {
         allocate_buffers(ctx);
-        allocate_textures(ctx);
+        allocate_objects(ctx, HLSL_TYPE_TEXTURE);
     }
     allocate_semantic_registers(ctx);
+    allocate_objects(ctx, HLSL_TYPE_SAMPLER);

if (ctx->result)
         return ctx->result;

[Giovanni Mascellani]

Signed-off-by: Giovanni Mascellani

On 09/11/21 14:26, Matteo Bruni wrote:
> From: Zebediah Figura
> Signed-off-by: Matteo Bruni
>
> Now it should actually apply on top of 219117.
>
> libs/vkd3d-shader/hlsl_codegen.c | 59 ++++++++++++++++++++++++--------
>   1 file changed, 44 insertions(+), 15 deletions(-)
>
> diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c
> index 0bf18c2b..7c0b0f95 100644
> --- a/libs/vkd3d-shader/hlsl_codegen.c
> +++ b/libs/vkd3d-shader/hlsl_codegen.c
> @@ -1202,60 +1202,88 @@ static void allocate_buffers(struct hlsl_ctx *ctx)
>       }
>   }
>
> -static const struct hlsl_ir_var *get_reserved_texture(struct hlsl_ctx *ctx, uint32_t index)
> +static const struct hlsl_ir_var *get_reserved_object(struct hlsl_ctx *ctx, char type, uint32_t index)
>   {
>       const struct hlsl_ir_var *var;
>   LIST_FOR_EACH_ENTRY(var, &ctx->extern_vars, const struct hlsl_ir_var, extern_entry)
>   {
> -
>    if (var->last_read && var->reg_reservation.type == 't' && var->reg_reservation.index == index)
> -
>    if (var->last_read && var->reg_reservation.type == type && var->reg_reservation.index == index)
>         return var;
> }
> return NULL;
> }
>
> -static void allocate_textures(struct hlsl_ctx *ctx)
> +static const struct object_type_info
>   {
> - enum hlsl_base_type type;
> - char *name;
> - char reg_name;
>
> +}
> +object_types[] =
> +{
> - { HLSL_TYPE_SAMPLER, "sampler", 's' },
> - { HLSL_TYPE_TEXTURE, "texture", 't' },
>
> +};
> -
>
> +static const struct object_type_info *get_object_type_info(enum hlsl_base_type type)
> +{
> - unsigned int i;
> -
> - for (i = 0; i < ARRAY_SIZE(object_types); ++i)
> -
>    if (type == object_types[i].type)
> -
>        return &object_types[i];
> -
> - WARN("No type info for object type %u.\n", type);
> - return NULL;
>
> +}
> -
>
> +static void allocate_objects(struct hlsl_ctx *ctx, enum hlsl_base_type type)
> +{
> -
>
> const struct object_type_info *type_info = get_object_type_info(type);
>  struct hlsl_ir_var *var;
>  uint32_t index = 0;
>
> LIST_FOR_EACH_ENTRY(var, &ctx->extern_vars, struct hlsl_ir_var, extern_entry)
>  {
>      if (!var->last_read || var->data_type->type != HLSL_CLASS_OBJECT
> -
>            || var->data_type->base_type != HLSL_TYPE_TEXTURE)
> -
>            || var->data_type->base_type != type)
>         continue;
>
> -
>    if (var->reg_reservation.type == 't')
> -
>    if (var->reg_reservation.type == type_info->reg_name)
>     {
> -
>        const struct hlsl_ir_var *reserved_texture = get_reserved_texture(ctx, var->reg_reservation.index);
> -
>        const struct hlsl_ir_var *reserved_object = get_reserved_object(ctx, type_info->reg_name,
> -
>                var->reg_reservation.index);
>
> -
>        if (reserved_texture && reserved_texture != var)
> -
>        if (reserved_object && reserved_object != var)
>         {
>             hlsl_error(ctx, var->loc, VKD3D_SHADER_ERROR_HLSL_OVERLAPPING_RESERVATIONS,
> -
>                    "Multiple textures bound to t%u.", var->reg_reservation.index);
> -
>            hlsl_note(ctx, reserved_texture->loc, VKD3D_SHADER_LOG_ERROR,
> -
>                    "Texture '%s' is already bound to t%u.", reserved_texture->name,
> -
>                    "Multiple %ss bound to %c%u.", type_info->name, type_info->reg_name,
>                     var->reg_reservation.index);
> -
>            hlsl_note(ctx, reserved_object->loc, VKD3D_SHADER_LOG_ERROR,
> -
>                    "Object '%s' is already bound to %c%u.", reserved_object->name,
> -
>                    type_info->reg_name, var->reg_reservation.index);
>         }
>
>         var->reg.id = var->reg_reservation.index;
>         var->reg.allocated = true;
> -
>        TRACE("Allocated reserved %s to t%u.\n", var->name, var->reg_reservation.index);
> -
>        TRACE("Allocated reserved %s to %c%u.\n", var->name, type_info->reg_name, var->reg_reservation.index);
>     }
>     else if (!var->reg_reservation.type)
>     {
> -
>        while (get_reserved_texture(ctx, index))
> -
>        while (get_reserved_object(ctx, type_info->reg_name, index))
>             ++index;
>
>         var->reg.id = index;
>         var->reg.allocated = true;
> -
>        TRACE("Allocated %s to t%u.\n", var->name, index);
> -
>        TRACE("Allocated %s to %c%u.\n", var->name, type_info->reg_name, index);
>         ++index;
>     }
>     else
>     {
>         hlsl_error(ctx, var->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_RESERVATION,
> -
>                "Textures must be bound to register type 't'.");
> -
>                "Object of type '%s' must be bound to register type '%c'.",
> -
>                type_info->name, type_info->reg_name);
>     }
> }
> }
>
> @@ -1376,9 +1404,10 @@ int hlsl_emit_dxbc(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *entry_fun
>       else
>       {
>           allocate_buffers(ctx);
> -
>    allocate_textures(ctx);
> -
>    allocate_objects(ctx, HLSL_TYPE_TEXTURE);
> }
> allocate_semantic_registers(ctx);
> -
>
> allocate_objects(ctx, HLSL_TYPE_SAMPLER);
>
> if (ctx->result)
>      return ctx->result;

[Zebediah Figura (she/her)]

On 11/9/21 07:26, Matteo Bruni wrote:
> From: Zebediah Figura
> Signed-off-by: Matteo Bruni
>
> Now it should actually apply on top of 219117.
>
> libs/vkd3d-shader/hlsl_codegen.c | 59 ++++++++++++++++++++++++--------
>   1 file changed, 44 insertions(+), 15 deletions(-)

Yeah, this does look better; I guess I didn't try hard enough. Note 
however there are a couple problems:

* this can't be used as-is for UAVs, because allocation doesn't start at 
zero;

* this complicates tbuffers as well, for similar reasons. (Actually 
things get worse with tbuffers, because now if you want to preserve that 
"already bound" message you need a way of checking for *both* kinds of 
objects.)

Neither one is particularly hard to solve, I guess, but I'm curious if 
you have a plan for them.
> +static void allocate_objects(struct hlsl_ctx *ctx, enum hlsl_base_type type)
> +{
> -
>
> const struct object_type_info *type_info = get_object_type_info(type);
>  struct hlsl_ir_var *var;
>  uint32_t index = 0;
>
> LIST_FOR_EACH_ENTRY(var, &ctx->extern_vars, struct hlsl_ir_var, extern_entry)
>  {
>      if (!var->last_read || var->data_type->type != HLSL_CLASS_OBJECT
> -
>            || var->data_type->base_type != HLSL_TYPE_TEXTURE)
> -
>            || var->data_type->base_type != type)
>         continue;
>
> -
>    if (var->reg_reservation.type == 't')
> -
>    if (var->reg_reservation.type == type_info->reg_name)
>     {
> -
>        const struct hlsl_ir_var *reserved_texture = get_reserved_texture(ctx, var->reg_reservation.index);
> -
>        const struct hlsl_ir_var *reserved_object = get_reserved_object(ctx, type_info->reg_name,
> -
>                var->reg_reservation.index);
>
> -
>        if (reserved_texture && reserved_texture != var)
> -
>        if (reserved_object && reserved_object != var)
>         {
>             hlsl_error(ctx, var->loc, VKD3D_SHADER_ERROR_HLSL_OVERLAPPING_RESERVATIONS,
> -
>                    "Multiple textures bound to t%u.", var->reg_reservation.index);
> -
>            hlsl_note(ctx, reserved_texture->loc, VKD3D_SHADER_LOG_ERROR,
> -
>                    "Texture '%s' is already bound to t%u.", reserved_texture->name,
> -
>                    "Multiple %ss bound to %c%u.", type_info->name, type_info->reg_name,
>                     var->reg_reservation.index);

At this rate I would just say "multiple objects", since it's not like 
there's extra semantic value in specifying the object type...
> -
>            hlsl_note(ctx, reserved_object->loc, VKD3D_SHADER_LOG_ERROR,
> -
>                    "Object '%s' is already bound to %c%u.", reserved_object->name,
> -
>                    type_info->reg_name, var->reg_reservation.index);
>         }
>
>         var->reg.id = var->reg_reservation.index;
>         var->reg.allocated = true;
> -
>        TRACE("Allocated reserved %s to t%u.\n", var->name, var->reg_reservation.index);
> -
>        TRACE("Allocated reserved %s to %c%u.\n", var->name, type_info->reg_name, var->reg_reservation.index);
>     }
>     else if (!var->reg_reservation.type)
>     {
> -
>        while (get_reserved_texture(ctx, index))
> -
>        while (get_reserved_object(ctx, type_info->reg_name, index))
>             ++index;
>
>         var->reg.id = index;
>         var->reg.allocated = true;
> -
>        TRACE("Allocated %s to t%u.\n", var->name, index);
> -
>        TRACE("Allocated %s to %c%u.\n", var->name, type_info->reg_name, index);
>         ++index;
>     }
>     else
>     {
>         hlsl_error(ctx, var->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_RESERVATION,
> -
>                "Textures must be bound to register type 't'.");
> -
>                "Object of type '%s' must be bound to register type '%c'.",
> -
>                type_info->name, type_info->reg_name);

...and use hlsl_type_to_string() here, and then you don't need 
type_info->name anymore.

[Matteo Bruni]

On Tue, Nov 9, 2021 at 9:50 PM Zebediah Figura (she/her)
 wrote:
> On 11/9/21 07:26, Matteo Bruni wrote:
> > From: Zebediah Figura
> > Signed-off-by: Matteo Bruni
> >
> > Now it should actually apply on top of 219117.
> >
> > libs/vkd3d-shader/hlsl_codegen.c | 59 ++++++++++++++++++++++++--------
> >   1 file changed, 44 insertions(+), 15 deletions(-)
>
> Yeah, this does look better; I guess I didn't try hard enough. Note
> however there are a couple problems:
> - this can't be used as-is for UAVs, because allocation doesn't start at
>
> zero;
> - this complicates tbuffers as well, for similar reasons. (Actually
>
> things get worse with tbuffers, because now if you want to preserve that
> "already bound" message you need a way of checking for *both* kinds of
> objects.)
>
> Neither one is particularly hard to solve, I guess, but I'm curious if
> you have a plan for them.

No, I admit I had a super quick skim to your branch for UAVs just to
make sure that it didn't go entirely in the opposite direction and
that's it.
> > +static void allocate_objects(struct hlsl_ctx *ctx, enum hlsl_base_type type)
> > +{
> > -
> >
> > const struct object_type_info *type_info = get_object_type_info(type);
> >  struct hlsl_ir_var *var;
> >  uint32_t index = 0;
> >
> > LIST_FOR_EACH_ENTRY(var, &ctx->extern_vars, struct hlsl_ir_var, extern_entry)
> >  {
> >      if (!var->last_read || var->data_type->type != HLSL_CLASS_OBJECT
> > -
> >            || var->data_type->base_type != HLSL_TYPE_TEXTURE)
> > -
> >            || var->data_type->base_type != type)
> >         continue;
> >
> > -
> >    if (var->reg_reservation.type == 't')
> > -
> >    if (var->reg_reservation.type == type_info->reg_name)
> >     {
> > -
> >        const struct hlsl_ir_var *reserved_texture = get_reserved_texture(ctx, var->reg_reservation.index);
> > -
> >        const struct hlsl_ir_var *reserved_object = get_reserved_object(ctx, type_info->reg_name,
> > -
> >                var->reg_reservation.index);
> >
> > -
> >        if (reserved_texture && reserved_texture != var)
> > -
> >        if (reserved_object && reserved_object != var)
> >         {
> >             hlsl_error(ctx, var->loc, VKD3D_SHADER_ERROR_HLSL_OVERLAPPING_RESERVATIONS,
> > -
> >                    "Multiple textures bound to t%u.", var->reg_reservation.index);
> > -
> >            hlsl_note(ctx, reserved_texture->loc, VKD3D_SHADER_LOG_ERROR,
> > -
> >                    "Texture '%s' is already bound to t%u.", reserved_texture->name,
> > -
> >                    "Multiple %ss bound to %c%u.", type_info->name, type_info->reg_name,
> >                     var->reg_reservation.index);
>
> At this rate I would just say "multiple objects", since it's not like
> there's extra semantic value in specifying the object type...
> > -
> >            hlsl_note(ctx, reserved_object->loc, VKD3D_SHADER_LOG_ERROR,
> > -
> >                    "Object '%s' is already bound to %c%u.", reserved_object->name,
> > -
> >                    type_info->reg_name, var->reg_reservation.index);
> >         }
> >
> >         var->reg.id = var->reg_reservation.index;
> >         var->reg.allocated = true;
> > -
> >        TRACE("Allocated reserved %s to t%u.\n", var->name, var->reg_reservation.index);
> > -
> >        TRACE("Allocated reserved %s to %c%u.\n", var->name, type_info->reg_name, var->reg_reservation.index);
> >     }
> >     else if (!var->reg_reservation.type)
> >     {
> > -
> >        while (get_reserved_texture(ctx, index))
> > -
> >        while (get_reserved_object(ctx, type_info->reg_name, index))
> >             ++index;
> >
> >         var->reg.id = index;
> >         var->reg.allocated = true;
> > -
> >        TRACE("Allocated %s to t%u.\n", var->name, index);
> > -
> >        TRACE("Allocated %s to %c%u.\n", var->name, type_info->reg_name, index);
> >         ++index;
> >     }
> >     else
> >     {
> >         hlsl_error(ctx, var->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_RESERVATION,
> > -
> >                "Textures must be bound to register type 't'.");
> > -
> >                "Object of type '%s' must be bound to register type '%c'.",
> > -
> >                type_info->name, type_info->reg_name);
>
> ...and use hlsl_type_to_string() here, and then you don't need
> type_info->name anymore.

Indeed, I'll make the change.