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:
- It is necessary to create constant nodes for the path, so, an instruction block pointer is required.
- 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;}