On 4/20/22 19:32, Francisco Casas wrote:
This works, but I'm not sure that it's my favourite organization. Without having tried it, what seems maybe more idiomatically structured is to bring the hlsl_new_store() call into this loop, not unlike the way it's done for constructors. You could still keep the recursive quality of the function for the benefit of structs and arrays, but effectively pass the store index as an in-out parameter instead of comps_count. I think this would also simplify error handling.
In more concrete terms, I'm envisioning something like:
static void initialize_var_components(struct hlsl_ctx *ctx, struct list *instrs, struct hlsl_ir_var *var, [in, out] unsigned int *store_index, struct hlsl_ir_node *instr)
which would be recursive, and called from what is currently initialize_numeric_var().
If I understand correctly, you are proposing to merge the "flatten" with the "initialize" functions. I tried it but I realized that it implies doing two types of recursion simultaneously, one on the "var" components and other on the "initializer->args"·s components.
Hmm, that's a good point, I hadn't considered that both sides would need to recurse.
Which is possible if we either:
- Explicitly keep the call stack of one of the recursions. I assume the argument index and
the call stack of "initializer->args".
- Having a way of quickly map the index of a component (I assume that's what you meant with
"store_index") to a register offset for any data type, or do a whole recursion each time we need that value.
I think the latter is fine, but keeping the code as it is is also fine.
I concluded that it is much more readable to keep the "flatten" separated from the "initialize" even if we mix those into one single function.
I still applied the formatting changes that Mystral suggested.