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.