[PATCH vkd3d 09/17] vkd3d-shader/hlsl: Replace register offsets with index paths in load initializations.

[Zebediah Figura]

On 7/14/22 20:23, Francisco Casas wrote:
> * The introduction of hlsl_new_store_index() was moved to another patch.
> * The introduction of hlsl_new_store_component() mas moved to another
>   patch.
> * The translation of hlsl_new_resource_load() to register offsets was
>   split from this patch.

Yeah, although I kind of meant moving them *before* this patch, along 
the principle of "abstract away the interface and then change the 
implementation". Probably not worth rewriting the series again at this 
point, though.

> Regarding the possibility of having a hlsl_deref_from_component_index() function, there are 2 inconveniences:
> 1. It is necessary to create constant nodes for the path, so, an instruction block pointer is required.
> 2. An additional hlsl_deref pointer is required for its use in hlsl_new_store_component(), since the lhs path needs to be prepend to the path to the component [1].
> 
> The signature of the function would end up like this:
> 
> static bool hlsl_deref_from_component_index(struct hlsl_ctx *ctx, struct hlsl_block *block,
>          struct hlsl_deref *deref, struct hlsl_ir_var *var, unsigned int index,
>          struct hlsl_deref *path_prefix, const struct vkd3d_shader_location *loc)
> 
> So in the end I think it is not a good idea to have hlsl_deref_from_component_index() directly.

> [1] There is also the alternative of introducing a hlsl_deref_concat() function that takes two derefs and retrieves a new one.

I don't think we'd need to pass the var *and* deref, would we?

Also: why do we need to pass a deref pointer to 
hlsl_new_store_component() in the first place? From 13/17 we only use it 
with variables directly. I guess the answer might be "consistency with 
hlsl_new_store_index()", which isn't a bad answer.

But, that said, I don't think that either of these are prohibitive, or 
make things worse than the alternative.

> +struct hlsl_type *hlsl_type_get_component_type(struct hlsl_ctx *ctx, struct hlsl_type *type,
> +        unsigned int index)
> +{
> +    while (!type_is_single_component(type))
> +        subtype_index_from_component_index(ctx, &type, &index);
> +
> +    return type;
> +}
> +
> +/* Returns the path of a given component within a type, given its index.
> + * *path_len will be set to the lenght of the path.
> + * Memory should be free afterwards.
> + */
> +static unsigned int *compute_component_path(struct hlsl_ctx *ctx, struct hlsl_type *type,
> +        unsigned int index, unsigned int *path_len)
> +{
> +    struct hlsl_type *path_type;
> +    unsigned int *path, path_index;
> +
> +    *path_len = 0;
> +    path_type = type;
> +    path_index = index;
> +    while (!type_is_single_component(path_type))
> +    {
> +        subtype_index_from_component_index(ctx, &path_type, &path_index);
> +        ++*path_len;
> +    }
> +    if (!(path = hlsl_alloc(ctx, *path_len * sizeof(unsigned int) + 1)))
> +        return NULL;
> +
> +    *path_len = 0;
> +    path_type = type;
> +    path_index = index;
> +    while (!type_is_single_component(path_type))
> +    {
> +        path[*path_len] = subtype_index_from_component_index(ctx, &path_type, &path_index);
> +        ++*path_len;
> +    }
> +
> +    return path;
> +}

Could we return path_type from this function, since we've already 
calculated it, and throw out hlsl_type_get_component_type() entirely?

> @@ -757,13 +948,21 @@ struct hlsl_ir_store *hlsl_new_store(struct hlsl_ctx *ctx, struct hlsl_ir_var *v
>           return NULL;
>   
>       init_node(&store->node, HLSL_IR_STORE, NULL, loc);
> -    store->lhs.var = var;
> +    init_deref(ctx, &store->lhs, var, 0);

Or hlsl_init_simple_deref_from_var(), here and in hlsl_new_load()?

>       hlsl_src_from_node(&store->lhs.offset, offset);
>       hlsl_src_from_node(&store->rhs, rhs);
>       store->writemask = writemask;
>       return store;
>   }