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

[Giovanni Mascellani]

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?
> -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.
>       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.
>   return load;
>
> }

Giovanni.