[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.

Giovanni Mascellani gmascellani at codeweavers.com
Wed Oct 20 06:51:09 CDT 2021


Signed-off-by: Giovanni Mascellani <gmascellani at codeweavers.com>
---
Again, some nitpicking about the commit message: like "DCE", "copy-prop" 
doesn't necessarily mean something to people who might like to learn 
about this code, and search engine do not seem to unilaterally associate 
it with "copy propagation". I think the few additional bytes are totally 
worth the value they bring.

On the other hand, I would suggest to keep the first line a bit shorter, 
because the general agreement is that it should be in the whereabouts of 
80 characters and user interfaces are usually tuned on that. Also, while 
in general I'm all for anthropomorphize whatever in the universe in 
colloquial language, I don't think it does well when clarity is needed. 
At the same time, I don't think that compilers are particularly bothered 
by having to synthesize temporaries: it's their job, and they'd better 
be fine with it, or find some other way to bring home their bacon.

Therefore I would suggest something like: "vkd3d-shader/hlsl: Don't 
synthesize unnecessary temporaries", and perhaps explain in greater 
detail in the commit message.

Il 15/10/21 23:54, Zebediah Figura ha scritto:
> 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);
> @@ -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);
> @@ -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);
> +
>       LIST_FOR_EACH_ENTRY(var, &ctx->globals->vars, struct hlsl_ir_var, scope_entry)
>       {
>           if (var->modifiers & HLSL_STORAGE_UNIFORM)
> 



More information about the wine-devel mailing list