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

[Matteo Bruni]

On Tue, Nov 9, 2021 at 9:50 PM Zebediah Figura (she/her)
<zfigura at codeweavers.com> wrote:
>
> On 11/9/21 07:26, Matteo Bruni wrote:
> > From: Zebediah Figura <zfigura at codeweavers.com>
> >
> > Signed-off-by: Matteo Bruni <mbruni at codeweavers.com>
> > ---
> > 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.