[PATCH vkd3d 05/12] vkd3d-shader/hlsl: Introduce add_load_index().

[Francisco Casas]

Hello,

On 05-07-22 07:26, Giovanni Mascellani wrote:
> Hi,
> 
> Il 01/07/22 23:24, Francisco Casas ha scritto:
>> +struct hlsl_ir_node *hlsl_new_offset_from_path_index(struct hlsl_ctx 
>> *ctx, struct hlsl_block *block,
>> +        struct hlsl_type *type, struct hlsl_ir_node *offset, struct 
>> hlsl_ir_node *idx,
>> +        const struct vkd3d_shader_location *loc)
> 
> Any reason why you're passing a struct hlsl_block and then copying the 
> instructions instead of the usual pattern of passing the instruction 
> list in depth?
> 

IIRC Zeb suggested it this way.

I think that using blocks is more versatile. We can introduce the 
generated instructions at the beginning of another instruction list, no 
just at the end. It may also help if we ever need to iterate these new 
instructions or insert them in the middle of the instruction list in 
some compilation passes.

No that those things are impossible to do with the "passing the 
instruction list" pattern, though.

>> -static struct hlsl_ir_load *add_load(struct hlsl_ctx *ctx, struct 
>> list *instrs, struct hlsl_ir_node *var_node,
>> -        struct hlsl_ir_node *offset, struct hlsl_type *data_type, 
>> const struct vkd3d_shader_location loc)
>> +static struct hlsl_ir_load *add_load_index(struct hlsl_ctx *ctx, 
>> struct list *instrs, struct hlsl_ir_node *var_node,
>> +        struct hlsl_ir_node *idx, const struct vkd3d_shader_location 
>> loc)
>>   {
>> -    struct hlsl_ir_node *add = NULL;
>> +    struct hlsl_type *elem_type;
>> +    struct hlsl_ir_node *offset;
>>       struct hlsl_ir_load *load;
>> -    struct hlsl_ir_var *var;
>> +    struct hlsl_block block;
>> +
>> +    elem_type = hlsl_get_type_from_path_index(ctx, 
>> var_node->data_type, idx);
>>       if (var_node->type == HLSL_IR_LOAD)
>>       {
>>           const struct hlsl_deref *src = &hlsl_ir_load(var_node)->src;
>> -        var = src->var;
>> -        if (src->offset.node)
>> -        {
>> -            if (!(add = hlsl_new_binary_expr(ctx, HLSL_OP2_ADD, 
>> src->offset.node, offset)))
>> -                return NULL;
>> -            list_add_tail(instrs, &add->entry);
>> -            offset = add;
>> -        }
>> +        if (!(offset = hlsl_new_offset_from_path_index(ctx, &block, 
>> var_node->data_type, src->offset.node, idx, &loc)))
>> +            return NULL;
>> +        list_move_tail(instrs, &block.instrs);
>> +
>> +        if (!(load = hlsl_new_load(ctx, src->var, offset, elem_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];
>> +        if (!(offset = hlsl_new_offset_from_path_index(ctx, &block, 
>> var_node->data_type, NULL, idx, &loc)))
>> +            return NULL;
>> +        list_move_tail(instrs, &block.instrs);
>> +
>>           sprintf(name, "<deref-%p>", var_node);
> 
> As for 04/12, I think the current trend is to use string buffers.
> 

Yep, I changed that for the next version here too.

>>           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, elem_type, loc)))
>> +            return NULL;
>> +        list_add_tail(instrs, &load->node.entry);
>>       }
>> -    if (!(load = hlsl_new_load(ctx, var, offset, data_type, loc)))
>> -        return NULL;
>> -    list_add_tail(instrs, &load->node.entry);
> 
> As I said, I liked it better when the hlsl_new_load() call wasn't 
> duplicated.
> 

... and here too.

>>       return load;
>>   }
> 
> Giovanni.
>