Hi,
On 10/11/21 10:57, Giovanni Mascellani wrote:
The CF part is probably going to be a lot trickier, but fortunately I think that this patch alone is the one that really matters. Frankly, if we were to take this patch, and then a second patch that does copy-prop on interior CF blocks with a fresh copy_propagation_state(), we might even cover enough that it's not even worrying about CF...
Mmh, note that you have to handle control flow in some way or another, you cannot just ignore it. This patch doesn't know about control flow, but the only way it can do it is by bailing out at the first control flow instruction in the program. It's not enough to ignore the control flow block and resume processing after it, because the inner block might invalidate some of your knowledge about the program state (in 3/5 patch this is done by copy_propagation_invalidate_from_block).
I think I misread your comment, even though my point is still valid: in your model, you have to refresh copy_propagation_state not only when you enter a block, but also when you leave it (actually, you might avoid refreshing when entering a conditional, though not when entering a loop). I think this means dropping quite a lot of useful information, and given that I don't find it particularly hard to avoid dropping it, it seems quite a waste.
Notice, in particular, that if you count on this copy propagation pass to fix texture loading, then you need to ensure, at a minimum, that all loads from a variable that is stored once at the beginning of the program (before any control flow) are correctly recognized and simplified. This does not happen with your proposal, I believe, while it happens with mine 3/5 (which currently has a bug which makes it unsound, but I will fix it in my next submission).
Well, technically at this point mine proposal wouldn't work as well, because it ignores object variables, but that can be addressed.
Giovanni.