[PATCH vkd3d 04/12] vkd3d-shader/hlsl: Introduce add_load_component() in hlsl.y.

[Francisco Casas]

Hello,

On 05-07-22 06:26, Giovanni Mascellani wrote:
> Hi,
> 
> about the patch subject, there is probably no real need to explicitly 
> mention hlsl.y.
> 

Right, I will change that.

> Il 01/07/22 23:24, Francisco Casas ha scritto:
>> +static struct hlsl_ir_load *add_load_component(struct hlsl_ctx *ctx, 
>> struct list *instrs, struct hlsl_ir_node *var_node,
>> +        unsigned int comp, const struct vkd3d_shader_location loc)
>> +{
>> +    struct hlsl_type *comp_type;
>> +    struct hlsl_ir_node *offset;
>> +    struct hlsl_ir_constant *c;
>> +    struct hlsl_ir_load *load;
>> +    unsigned int comp_offset;
>> +    struct hlsl_ir_var *var;
>> +
>> +    comp_offset = hlsl_compute_component_offset(ctx, 
>> var_node->data_type, comp, &comp_type);
>> +
>> +    if (!(c = hlsl_new_uint_constant(ctx, comp_offset, &loc)))
>> +        return NULL;
>> +    list_add_tail(instrs, &c->node.entry);
>> +
>> +    offset = &c->node;
>> +
>> +    if (var_node->type == HLSL_IR_LOAD)
>> +    {
>> +        const struct hlsl_deref *src = &hlsl_ir_load(var_node)->src;
>> +        struct hlsl_ir_node *add;
>> +
>> +        var = src->var;
>> +        if (src->offset.node)
>> +        {
>> +            if (!(add = hlsl_new_binary_expr(ctx, HLSL_OP2_ADD, 
>> src->offset.node, &c->node)))
>> +                return NULL;
>> +            list_add_tail(instrs, &add->entry);
>> +            offset = add;
>> +        }
>> +
>> +        if (!(load = hlsl_new_load(ctx, var, offset, comp_type, loc)))
>> +            return NULL;
>> +        list_add_tail(instrs, &load->node.entry);
>> +    }
>> +    else
>> +    {
>> +        struct hlsl_ir_store *store;
>> +        struct hlsl_ir_var *var;
>> +        char name[27];
>> +
>> +        sprintf(name, "<deref-%p>", var_node);
> 
> I think the new trend is to use string buffers. See how this is done in 
> intrinsic_mul(), for example.
> 

I see. I will use string buffers in the next version.

> (if you ask me, I would also stop using global static counters, but 
> that's not happening yet; also, it's just a cosmetic issue)
> 
>> +        if (!(var = hlsl_new_synthetic_var(ctx, name, 
>> var_node->data_type, var_node->loc)))
>> +            return NULL;
>> +
>> +        if (!(store = hlsl_new_simple_store(ctx, var, var_node)))
>> +            return NULL;
>> +        list_add_tail(instrs, &store->node.entry);
>> +
>> +        if (!(load = hlsl_new_load(ctx, var, offset, comp_type, loc)))
>> +            return NULL;
>> +        list_add_tail(instrs, &load->node.entry);
> 
> This last block is identical in both branches, and it remains mostly 
> identical also in later patches. Maybe it could be brought outside of 
> the if, like it is in add_load()?
> 

Good observation. I changed this in add_load_component() and 
add_load_index() for the next version. It requires moving some 
declarations to the outermost scope though.

I realized that here we have two "struct hlsl_ir_var *var;" declarations 
in different scopes too. I got rid of that too.

> Thanks, Giovanni.
>