[PATCH vkd3d 02/17] vkd3d-shader/hlsl: Handle branches in copy propagation.

[Zebediah Figura]

On 12/3/21 03:28, Giovanni Mascellani wrote:
> Hi,
> 
> On 01/12/21 17:14, Matteo Bruni wrote:
>> changelog: Continue copy propagation after encountering an IF, rework to
>> store the copy_propagation_state structs on the stack and search
>> recursively
> 
> I still don't like these changes, they make the code more complicated 
> than it should be, but I'll live with them.
> 
> However, I think you're introducing some missed optimizations with this 
> specific patch. For example, take this code:
> 
> float2 a;
> a.x = 1.0;
> if (condition)
> {
>      a.y = 2.0;
>      do_something(a.x);
> }
> 
> You would expect a.x replaced with 1.0 in do_something(), but I don't 
> think that's happening in you implementation: inside the conditional a 
> new copy_propagation_var_def is created for variable a, which masks the 
> previous knowledge about a.x being 1.0. This can probably be fixed in 
> copy_propagation_create_var_def in the missing entry case by first doing 
> a lookup on the lower frames and using that result, if it exists, to 
> initialize the new copy_propagation_var_def.
>
Actually, it's worse than that; this patch seems to be incorrectly 
optimizing the following code:

--

bool b;

float4 main() : sv_target
{
     float2 a;
     a.x = 1.0;
     if (b)
     {
         a.y = 2.0;
         a.x += 3;
     }
     return float4(a, 0.0, 0.0);
}

--

The intermediate IR dump says:

  2:       bool | b
  3:      float | 1.00000000e+00
  4:            | = (a.x @3)
  5:            | if (@2) {
  6:      float | 2.00000000e+00
  7:            | = (a.y @6)
  8:     float2 | @6.xx
  9:     float1 | @8.x
10:      float | 3.00000000e+00
11:      float | + (@9 @10 )
12:            | = (a.x @11)
                  } else {
                  }
13:     float2 | a
14:      float | 0.00000000e+00
15:      float | 0.00000000e+00
16:            | = (<constructor-0>.xy @13)
17:            | = (<constructor-0>.z @14)
18:            | = (<constructor-0>.w @15)
19:     float4 | <constructor-0>
20:            | return
21:            | = (<output-sv_target0> @19)

Note instruction 8 here, which is probably supposed to be @3.xx rather 
than @6.xx.