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