[PATCH vkd3d 6/7] vkd3d-shader/hlsl: Don't bother synthesizing a temp for input or output variables if they're not written to or read from, respectively.

[Matteo Bruni]

On Fri, Oct 15, 2021 at 11:54 PM Zebediah Figura
<zfigura at codeweavers.com> wrote:
>
> As a cheap substitute for actual variable copy-prop.
>
> Signed-off-by: Zebediah Figura <zfigura at codeweavers.com>
> ---
>  libs/vkd3d-shader/hlsl_codegen.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>
> diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c
> index df10ca272..495ba7c47 100644
> --- a/libs/vkd3d-shader/hlsl_codegen.c
> +++ b/libs/vkd3d-shader/hlsl_codegen.c
> @@ -34,6 +34,13 @@ static void prepend_uniform_copy(struct hlsl_ctx *ctx, struct list *instrs, stru
>      /* Use the synthetic name for the temp, rather than the uniform, so that we
>       * can write the uniform name into the shader reflection data. */
>
> +    if (!temp->first_write)
> +    {
> +        temp->is_uniform = 1;
> +        list_add_tail(&ctx->extern_vars, &temp->extern_entry);
> +        return;
> +    }
> +
>      if (!(uniform = hlsl_new_var(ctx, temp->name, temp->data_type, temp->loc, NULL, 0, &temp->reg_reservation)))
>          return;
>      list_add_before(&temp->scope_entry, &uniform->scope_entry);

Technically this hunk is not covered by the patch subject.

I'm not sure I love these checks being in the functions rather than in
the caller. It might just be about function naming... I guess what's
bugging me is that, for "good" uniforms, prepend_uniform_copy() will
NOT prepend any copy but still do something significant.

> @@ -67,6 +74,14 @@ static void prepend_input_copy(struct hlsl_ctx *ctx, struct list *instrs, struct
>      struct hlsl_ir_load *load;
>      struct hlsl_ir_var *input;
>
> +    /* We still need to split up non-vector types. */
> +    if (!var->first_write && var->data_type->type <= HLSL_CLASS_VECTOR)
> +    {
> +        var->is_input_semantic = 1;
> +        list_add_tail(&ctx->extern_vars, &var->extern_entry);
> +        return;
> +    }
> +
>      if (!(name = hlsl_get_string_buffer(ctx)))
>          return;
>      vkd3d_string_buffer_printf(name, "<input-%s%u>", semantic->name, semantic->index);

It's not obvious what splitting up input matrices (which is not there
at all at the moment) has to do with adding a copy from an input
variable.

> @@ -138,6 +153,14 @@ static void append_output_copy(struct hlsl_ctx *ctx, struct list *instrs, struct
>      struct hlsl_ir_var *output;
>      struct hlsl_ir_load *load;
>
> +    /* We still need to split up non-vector types. */
> +    if (!var->last_read && var->data_type->type <= HLSL_CLASS_VECTOR)
> +    {
> +        var->is_output_semantic = 1;
> +        list_add_tail(&ctx->extern_vars, &var->extern_entry);
> +        return;
> +    }
> +
>      if (!(name = hlsl_get_string_buffer(ctx)))
>          return;
>      vkd3d_string_buffer_printf(name, "<output-%s%u>", semantic->name, semantic->index);
> @@ -1306,6 +1329,12 @@ int hlsl_emit_dxbc(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *entry_fun
>
>      list_move_head(&body->instrs, &ctx->static_initializers);
>
> +    /* Do an initial liveness pass for externs so that we don't create
> +     * unnecessary temps. Note that we might modify the instruction stream
> +     * during the pass, but we're only checking whether a variable was read to
> +     * or written from at all, so it's okay. */
> +    compute_liveness(ctx, entry_func);

We shouldn't be modifying the instructions (aside from liveness data,
of course) in compute_liveness(). I guess you mean that we can safely
keep using liveness data that potentially becomes somewhat invalid
during / after calling prepend_uniform_copy() /
prepend_input_var_copy() / append_output_var_copy() but I'm having a
hard time parsing the comment.