[PATCH vkd3d v6 5/6] vkd3d-shader/hlsl: Handle conditionals in copy propagation.

[Giovanni Mascellani]

Hi,

On 18/11/21 05:05, Zebediah Figura (she/her) wrote:
> I don't think you even need to add any function parameters, do you? 
> Instead you do something like:
> 
> struct copy_propagation_state
> {
>      struct rbtree variables;
>      // if we want to avoid duplicating the contents:
>      const struct copy_propagation_state *parent;
> };
> 
> static bool copy_propagation_handle_if(struct hlsl_ctx *ctx,
>          struct hlsl_ir_if *iff, struct copy_propagation_state *parent)
> {
>      struct copy_propagation_state state;
> 
>      rb_init(&state.variables);
> 
>      // duplicate parent->variables into state.variables...
>      // or, alternatively:
>      state.parent = parent;
> }
> 
> Same number of parameters as we currently have.
> 
> I'm not strongly attached to this solution, but I will say that coming 
> from a probably neutral perspective, it doesn't seem any more (or less?) 
> complex than what you have.

Oh, I see what you mean.

Personally, I still prefer my solution. It feels more idiomatic, you 
don't have to couple too much the stack in the compiler with the stack 
in the compiled program, so if you don't have a strong preference I 
would keep my way.

>>> One potential alternative that occurred to me: instead of duplicating 
>>> the whole state, we can search backwards in each parent scope in 
>>> copy_propagation_get_variable(). Notably, in the case of a loop, we'd 
>>> hit NULL before a "real" variable, and return NULL.
>>>
>>> Assuming that works it'd reduce the amount of allocation we have to do.
>>
>> I think it would work (I think I considered this approach at some 
>> point) and it would reduce the amount of allocation we do, but it 
>> would increase the amount of work that programmers have to do to write 
>> and maintain the code. Depending on the variable usage patterns, it 
>> could also increase the running time of the program. For these two 
>> reasons, mostly the first one, I don't think it is a good idea until 
>> stronger data emerges.
> 
> Well, coming from a perspective that hasn't been working with either 
> variation, I'd say that they seem about equally natural to me. I don't 
> strongly prefer recursive search over duplication, though, so I don't 
> really intend to fight over this point.

Here too I'd like to keep my way if you don't have a strong position on 
this.

> Wrt CPU patterns, I'd be really surprised if searching parent scopes 
> ends up being more CPU-intensive than duplicating scope contents. You'd 
> have to be accessing each variable enough times to offset the cost of 
> memory allocation.

On the other hand, each variable access with your solution requires many 
RB tree lookups, so the balance really depends on the usage patterns.

Giovanni.