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.