Re: [PATCH vkd3d v2 1/3] vkd3d-shader/hlsl: Handle branches in copy propagation.

[Zebediah Figura (she/her)]

On 1/20/22 05:23, Giovanni Mascellani wrote:
> @@ -307,15 +316,23 @@ static void copy_propagation_var_def_destroy(struct rb_entry *entry, void *conte
>       vkd3d_free(var_def);
>   }
>
> -static struct copy_propagation_var_def *copy_propagation_get_var_def(struct copy_propagation_state *state,
> -
>    struct hlsl_ir_var *var)
>
> +static struct copy_propagation_value *copy_propagation_get_value(struct copy_propagation_state *state,
> -
>    struct hlsl_ir_var *var, unsigned component)
> {
> - struct rb_entry *entry = rb_get(&state->var_defs, var);
> - for (; state; state = state->parent)
> - {
> -
>    struct rb_entry *entry = rb_get(&state->var_defs, var);
> -
>    if (entry)
> -
>    {
> -
>        struct copy_propagation_var_def *var_def = RB_ENTRY_VALUE(entry, struct copy_propagation_var_def, entry);
> -
>        assert(component < var_def->var->data_type->reg_size);
>
> - if (entry)
> -
>    return RB_ENTRY_VALUE(entry, struct copy_propagation_var_def, entry);
> - else
> -
>    return NULL;
> -
>        if (var_def->values[component].state != VALUE_STATE_NOT_WRITTEN)

Why not just check for VALUE_STATE_STATICALLY_WRITTEN?
> -
>            return &var_def->values[component];
> -
>    }
> -
>
> }
> -
> -
>
> return NULL;
> }
>
> static struct copy_propagation_var_def *copy_propagation_create_var_def(struct hlsl_ctx *ctx,
>
> @@ -339,10 +356,35 @@ static struct copy_propagation_var_def *copy_propagation_create_var_def(struct h
>       return var_def;
>   }
>
> +static void copy_propagation_invalidate_variable(struct copy_propagation_var_def *var_def,
> -
>    unsigned offset, unsigned char writemask)
>
> +{
> - unsigned i;
> -
> - TRACE("Invalidate variable %s[%u]%s.\n", var_def->var->name, offset, debug_hlsl_writemask(writemask));
> -
> - for (i = 0; i < 4; ++i)
> - {
> -
>    if (writemask & (1u << i))
> -
>    {
> -
>        memset(&var_def->values[offset + i], 0, sizeof(var_def->values[offset + i]));

This memset() doesn't seem necessary anymore.
> -
>        var_def->values[offset + i].state = VALUE_STATE_DYNAMICALLY_WRITTEN;
> -
>    }
> - }
>
> +}
> - static void copy_propagation_invalidate_whole_variable(struct copy_propagation_var_def *var_def)
> {
> - unsigned i;
> -
> TRACE("Invalidate variable %s.\n", var_def->var->name);
> -
> memset(var_def->values, 0, sizeof(*var_def->values) * var_def->var->data_type->reg_size);

Note that you don't need this memset() anymore either.
> -
> - for (i = 0; i < var_def->var->data_type->reg_size; ++i)
> - {
> -
>    var_def->values[i].state = VALUE_STATE_DYNAMICALLY_WRITTEN;
> - }
> }

...
> +static void copy_propagation_invalidate_from_block(struct hlsl_ctx *ctx, struct copy_propagation_state *state,
> -
>    struct hlsl_block *block)
>
> +{
> - struct hlsl_ir_node *instr;
> -
> - LIST_FOR_EACH_ENTRY(instr, &block->instrs, struct hlsl_ir_node, entry)
> - {
> -
>    switch (instr->type)
> -
>    {
> -
>        case HLSL_IR_STORE:
> -
>        {
> -
>            struct hlsl_ir_store *store = hlsl_ir_store(instr);
> -
>            struct copy_propagation_var_def *var_def;
> -
>            struct hlsl_deref *lhs = &store->lhs;
> -
>            struct hlsl_ir_var *var = lhs->var;
> -
>            unsigned int offset;
> -
> -
>            if (!(var_def = copy_propagation_create_var_def(ctx, state, var)))
> -
>                continue;
> -
> -
>            if (hlsl_offset_from_deref(lhs, &offset))
> -
>                copy_propagation_invalidate_variable(var_def, offset, store->writemask);
> -
>            else
> -
>                copy_propagation_invalidate_whole_variable(var_def);
> -
> -
>            break;
> -
>        }
> -
> -
>        case HLSL_IR_IF:
> -
>        {
> -
>            struct hlsl_ir_if *iff = hlsl_ir_if(instr);
> -
> -
>            copy_propagation_invalidate_from_block(ctx, state, &iff->then_instrs);
> -
>            copy_propagation_invalidate_from_block(ctx, state, &iff->else_instrs);
> -
> -
>            break;
> -
>        }
> -
> -
>        case HLSL_IR_LOOP:
> -
>        {
> -
>            struct hlsl_ir_loop *loop = hlsl_ir_loop(instr);
> -
> -
>            copy_propagation_invalidate_from_block(ctx, state, &loop->body);
> -
> -
>            break;
> -
>        }
> -
> -
>        default:
> -
>            break;
> -
>    }
> - }
>
> +}

As an alternative, you could iterate over the whole state, marking the 
parent as "dynamically written" wherever the child state is written.