[PATCH vkd3d 1/3] vkd3d-shader/hlsl: Handle branches in copy propagation.

[Zebediah Figura (she/her)]

On 12/20/21 02:51, Giovanni Mascellani wrote:
> Hi,
> 
> On 17/12/21 18:59, Zebediah Figura (she/her) wrote:
>>> A new var_def is now initialized to the content of the same var_def
>>> in earlier stack frames.
>>
>> I'm not a big fan of this, because now it feels like it's half one 
>> thing and half the other.
> 
> Neither I like it (my preferred solution is still my original proposal), 
> but it was necessary to fix Matteo's changes, which hopefully you don't 
> like either (since you found they are buggy). For the pars construens, 
> which solution would you prefer now? Matteo, which alternative do you 
> prefer?
> 
>> Something that just occurred to me: could we handle this by instead 
>> just comparing the var_def structures from the parent and child state, 
>> and invalidating where they don't match?
>>
>> Alternatively, probably simpler: could we just recursively invalidate 
>> parents in copy_propagation_record_store()?
> 
> I guess we could.
> 
> I don't like it because it makes it harder for me to convince myself 
> that there is a proof of the correctness of the algorithm, and we have 
> just seen that when it is harder to prove the correctness of an 
> algorithm it is also easier that there are actual bugs; and on the other 
> hand I see no points in favor in doing the changes (except perhaps for, 
> in the case of the recursive var_def search, vague speculations about 
> the fact that it might be a little faster.
> 
> Do you think that changing this would make it easier for you to check 
> that the code is correct? Which other advantages do you see for the 
> change your are proposing?

I'll admit, I still am inclined to prefer not duplicating variable 
state, and I don't share your concern about code correctness, even 
though we've been bitten once already.

I suppose in that case the solution would be to store some extra bit of 
state along with the hlsl_ir_node pointer, say "bool written;".