Re: [PATCH vkd3d 5/5] vkd3d-shader: Allocate constant buffers.

[Matteo Bruni]

On Thu, Jun 24, 2021 at 7:15 AM Zebediah Figura  wrote:
> Signed-off-by: Zebediah Figura
>
> libs/vkd3d-shader/hlsl.c                 |  21 +++--
>  libs/vkd3d-shader/hlsl.h                 |   5 ++
>  libs/vkd3d-shader/hlsl_codegen.c         | 101 +++++++++++++++++++++++
>  libs/vkd3d-shader/vkd3d_shader_private.h |   2 +
>  4 files changed, 121 insertions(+), 8 deletions(-)
> diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c
> index 2768f8ac..6336ddfc 100644
> --- a/libs/vkd3d-shader/hlsl_codegen.c
> +++ b/libs/vkd3d-shader/hlsl_codegen.c
> @@ -1115,6 +1115,105 @@ static void allocate_semantic_registers(struct hlsl_ctx *ctx)
>      }
>  }
>
> +static const struct hlsl_buffer *get_reserved_buffer(struct hlsl_ctx *ctx, uint32_t index)
> +{
> - const struct hlsl_buffer *buffer;
> -
> - LIST_FOR_EACH_ENTRY(buffer, &ctx->buffers, const struct hlsl_buffer, entry)
> - {
> -
>    if (buffer->used_size && buffer->reservation.type == 'b' && buffer->reservation.index == index)
> -
>        return buffer;
> - }
> - return NULL;
>
> +}
> -
>
> +static void calculate_buffer_offset(struct hlsl_ir_var *var)
> +{
> - struct hlsl_buffer *buffer = var->buffer;
> -
> - buffer->size = hlsl_type_get_sm4_offset(var->data_type, buffer->size);
> -
> - var->buffer_offset = buffer->size;
> - TRACE("Allocated buffer offset %u to %s.\n", var->buffer_offset, var->name);
> - buffer->size += var->data_type->reg_size;
> - if (var->last_read)
> -
>    buffer->used_size = buffer->size;
>
> +}
> -
>
> +static void allocate_buffers(struct hlsl_ctx *ctx)
> +{
> - struct hlsl_buffer *buffer, *params_buffer;
> - struct hlsl_ir_var *var;
> - uint32_t index = 0;
> -
> - if (!(params_buffer = hlsl_new_buffer(ctx, HLSL_BUFFER_CONSTANT,
> -
>        hlsl_strdup(ctx, "$Params"), NULL, ctx->location)))
> -
>    return;
> -
> - /* The $Globals and $Params buffers should be allocated first, before all
> -
> * explicit buffers. */
> - list_remove(&params_buffer->entry);
> - list_add_head(&ctx->buffers, &params_buffer->entry);
> - list_remove(&ctx->globals_buffer->entry);
> - list_add_head(&ctx->buffers, &ctx->globals_buffer->entry);

I find the split in the creation of $Globals and $Params a tiny bit
annoying. Right now $Globals is created in hlsl_ctx_init() (which also
means that there should be no need to explicitly put it at the top of
the buffers list here AFAIU) while $Params is created right here,
after parsing. If the point is just to make sure that $Params is the
second buffer in the list, list_add_after() should make it a bit more
compact and explicit. In theory you could add an assert() if you want
to make sure that $Globals is at the head of the list, although
probably it would be even better if both these special buffers were
created together, in the same place.
Unless I'm missing something?