[PATCH vkd3d 09/12] vkd3d-shader/hlsl: Flatten initializers.

[Francisco Casas]

Hello, just for the record

April 21, 2022 3:02 PM, "Zebediah Figura" <zfigura at codeweavers.com> wrote:

> 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.
> 

In the end I tried this and it turned out into a nice patch that supports all complex
initializers. So it is better not to accept 09/12, 10/12, 11/12, and 12/12 yet.

>> 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 no longer agree with my previous self on this).

>> I still applied the formatting changes that Mystral suggested.


Best regards,
Francisco.