[PATCH 0/1] MR674: tests: Test how constant folding works on SM1.
While reviewing !671 I wrote a few tests that are probably not too fundamental, but since now they exist it makes sense to keep them. !671 doesn't fix nor break any of them, anyway. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/674
From: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- Makefile.am | 1 + tests/hlsl/sm1-const-folding.shader_test | 147 +++++++++++++++++++++++ 2 files changed, 148 insertions(+) create mode 100644 tests/hlsl/sm1-const-folding.shader_test diff --git a/Makefile.am b/Makefile.am index cfb225f9c..9e761ef32 100644 --- a/Makefile.am +++ b/Makefile.am @@ -170,6 +170,7 @@ vkd3d_shader_tests = \ tests/hlsl/side-effects.shader_test \ tests/hlsl/sign.shader_test \ tests/hlsl/single-numeric-initializer.shader_test \ + tests/hlsl/sm1-const-folding.shader_test \ tests/hlsl/sm6-ternary.shader_test \ tests/hlsl/sm6-uav-rwtexture.shader_test \ tests/hlsl/smoothstep.shader_test \ diff --git a/tests/hlsl/sm1-const-folding.shader_test b/tests/hlsl/sm1-const-folding.shader_test new file mode 100644 index 000000000..055b6a0cd --- /dev/null +++ b/tests/hlsl/sm1-const-folding.shader_test @@ -0,0 +1,147 @@ +% Test the interaction between integer constant folding and the fact +% that integers are secretly considered floats in SM1-3 + +[pixel shader] +float4 main() : SV_TARGET +{ + uint x = 3000000000u; + return float4(x, 0.0, 0.0, 0.0); +} + +[test] +draw quad +probe all rgba (3.0e+009, 0.0, 0.0, 0.0) + +% The uint value is never casted to int on SM1-3, just casted to float and treated as such +[pixel shader] +float4 main() : SV_TARGET +{ + int x = 3000000000u; + return float4(x, 0.0, 0.0, 0.0); +} + +[test] +draw quad +if(sm<4) todo probe all rgba (3.0e+009, 0.0, 0.0, 0.0) +if(sm>=4) probe all rgba (-1.29496730e+009, 0.0, 0.0, 0.0) + +% On SM1-5 (FXC) the literal is considered signed independently of its value and casted to uint; +% on SM1-3 compilation fails because the uint it would be negative +[pixel shader fail(sm<4) todo(sm<4)] +float4 main() : SV_TARGET +{ + uint x = 3000000000; + return float4(x, 0.0, 0.0, 0.0); +} + +[test] +draw quad +if(sm>=4) probe all rgba (3.0e+009, 0.0, 0.0, 0.0) + +[pixel shader] +float4 main() : SV_TARGET +{ + int x = 3000000000; + return float4(x, 0.0, 0.0, 0.0); +} + +[test] +draw quad +probe all rgba (-1.29496730e+009, 0.0, 0.0, 0.0) + +% These two make sense +[pixel shader] +float4 main() : SV_TARGET +{ + return float4(3000000000u, 0.0, 0.0, 0.0); +} + +[test] +draw quad +probe all rgba (3.0e+009, 0.0, 0.0, 0.0) + +% Unsuffixed literals can instead be considered unsigned on SM6 +[pixel shader] +float4 main() : SV_TARGET +{ + return float4(3000000000, 0.0, 0.0, 0.0); +} + +[test] +draw quad +if(sm<6) probe all rgba (-1.29496730e+009, 0.0, 0.0, 0.0) +if(sm>=6) probe all rgba (3.0e+009, 0.0, 0.0, 0.0) + +% Constant folding is done with uint semantics before converting to float; +% so constant folding seems to happen before converting integer operations to floats +[pixel shader] +float4 main() : SV_TARGET +{ + uint x = 3000000000u + 3000000000u; + return float4(x, 0.0, 0.0, 0.0); +} + +[test] +draw quad +probe all rgba (1.70503270e+009, 0.0, 0.0, 0.0) + +% SM1-3 complains that uint values must be positive, but it's not clear why this is a problem here: +% the sum should be done before casting to uint, and the result of the int sum should be positive, +% so it seems that the declared type of x means something on how constant folding is done. +% I'm not sure what's happening for SM4 anyway +[pixel shader fail(sm<4) todo(sm<4)] +float4 main() : SV_TARGET +{ + uint x = 3000000000 + 3000000000; + return float4(x, 0.0, 0.0, 0.0); +} + +[test] +draw quad +todo probe all rgba (2.14748365e+009, 0.0, 0.0, 0.0) + +% Again, no idea of what's happening here +[pixel shader] +float4 main() : SV_TARGET +{ + int x = 3000000000 + 3000000000; + return float4(x, 0.0, 0.0, 0.0); +} + +[test] +draw quad +todo probe all rgba (-2.14748365e+009, 0.0, 0.0, 0.0) + +% This makes sense +[pixel shader] +float4 main() : SV_TARGET +{ + return float4(3000000000u + 3000000000u, 0.0, 0.0, 0.0); +} + +[test] +draw quad +probe all rgba (1.70503270e+009, 0.0, 0.0, 0.0) + +% This does not again +[pixel shader] +float4 main() : SV_TARGET +{ + return float4(3000000000 + 3000000000, 0.0, 0.0, 0.0); +} + +[test] +draw quad +todo probe all rgba (-2.14748365e+009, 0.0, 0.0, 0.0) + +% Explicit float literals use float semantics, at least +[pixel shader] +float4 main() : SV_TARGET +{ + float x = 3000000000.0 + 3000000000.0; + return float4(x, 0.0, 0.0, 0.0); +} + +[test] +draw quad +probe all rgba (6.0e+009, 0.0, 0.0, 0.0) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/674
This merge request was approved by Giovanni Mascellani. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/674
Francisco Casas (@fcasas) commented about tests/hlsl/sm1-const-folding.shader_test:
+ +% The uint value is never casted to int on SM1-3, just casted to float and treated as such +[pixel shader] +float4 main() : SV_TARGET +{ + int x = 3000000000u; + return float4(x, 0.0, 0.0, 0.0); +} + +[test] +draw quad +if(sm<4) todo probe all rgba (3.0e+009, 0.0, 0.0, 0.0) +if(sm>=4) probe all rgba (-1.29496730e+009, 0.0, 0.0, 0.0) + +% On SM1-5 (FXC) the literal is considered signed independently of its value and casted to uint; +% on SM1-3 compilation fails because the uint it would be negative "the uint would be negative"?
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/674#note_62433
Francisco Casas (@fcasas) commented about tests/hlsl/sm1-const-folding.shader_test:
+[test] +draw quad +if(sm>=4) probe all rgba (3.0e+009, 0.0, 0.0, 0.0) + +[pixel shader] +float4 main() : SV_TARGET +{ + int x = 3000000000; + return float4(x, 0.0, 0.0, 0.0); +} + +[test] +draw quad +probe all rgba (-1.29496730e+009, 0.0, 0.0, 0.0) + +% These two make sense which two things? The last two tests?
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/674#note_62434
This merge request was approved by Francisco Casas. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/674
Interesting experiments! This is making me having two separate hypothesis for SM1: A) Constant propagation skips casts from int to uint and vice-versa, which explains the result of the second test. B) Native must be doing something similar to my pass from !671 2/3 but before constant propagation. This means that constant folding casts the constants to float before performing the operation, and then casts the result back to the expected type. So in this test: ```hlsl [pixel shader fail(sm<4) todo(sm<4)] float4 main() : SV_TARGET { uint x = 3000000000 + 3000000000; return float4(x, 0.0, 0.0, 0.0); } [test] draw quad todo probe all rgba (2.14748365e+009, 0.0, 0.0, 0.0) ``` - Each `3000000000` is stored as `int`, so each overflows to `-1294967296`. - In the presence of arithmetic operators, constant folding turns them to float. - The `+` is performed with floating-point arithmetic resulting in `-1294967296.0f - 1294967296.0f = -2589934592.0f`. - The result is turned back to int which in C results in `2147483648` (INT_MAX) because it cannot be represented as int (yep, it is UB). --- I am approving the merge request, check my minor comment nitpicks if you got the chance before it gets merged. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/674#note_62435
On Fri Feb 23 19:36:48 2024 +0000, Francisco Casas wrote:
Interesting experiments! This is making me having two separate hypothesis for SM1: A) Constant propagation skips casts from int to uint and vice-versa, which explains the result of the second test. B) Native must be doing something similar to my pass from !671 2/3 but before constant propagation. This means that constant folding casts the constants to float before performing the operation, and then casts the result back to the expected type. So in this test: ```hlsl [pixel shader fail(sm<4) todo(sm<4)] float4 main() : SV_TARGET { uint x = 3000000000 + 3000000000; return float4(x, 0.0, 0.0, 0.0); } [test] draw quad todo probe all rgba (2.14748365e+009, 0.0, 0.0, 0.0) ``` - Each `3000000000` is stored as `int`, so each overflows to `-1294967296`. - In the presence of arithmetic operators, constant folding turns them to float. - The `+` is performed with floating-point arithmetic resulting in `-1294967296.0f - 1294967296.0f = -2589934592.0f`. - The result is turned back to int which in C results in `2147483648` (INT_MAX) because it cannot be represented as int (yep, it is UB). --- I am approving the merge request, check my minor comment nitpicks if you got the chance before it gets merged. Please disregard my hypothesis B. I tried that and it didn't work. In fact you even mentioned:
% Constant folding is done with uint semantics before converting to float;
% so constant folding seems to happen before converting integer operations to floats
but maybe there is a lot more going on, for instance, notice this shader: https://shader-playground.timjones.io/df675e96acb6d16bb77dc0ebdf69dfb2 Constant folding is even preempting catastrophic cancellation. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/674#note_62440
I did some additional testing, and it's more complicated than that. I think I alluded to this elsewhere, but the native compiler has a concept of an "ambiguous" integer type, which is neither int nor uint. You can tell this by doing something like ``` float func(int x) { return 1; } float func(uint x) { return 2; } float4 main() : SV_TARGET { return func(1); } ``` Ambiguous ints are automatically promoted when in an expression with other types, and can of course be implicitly cast, but an expression comprised only of ambiguous ints is still ambiguous. Different integer constants have different types: uint: 1u, 0x1, 01, 0x0, 4294967295u, 4294967296u, 4294967296lu int: 3000000000, -3000000000, 2147483648, -2147483648 ambiguous: 1, 0, 00, 000, -0, -1, 2147483647, -2147483647, 4294967296l, 1 + 2, 1 / 2 empty: 4294967296 Note "empty": that last token lexes as if it were a space. Fun! uint adds with overflow, as you'd expect. Int and ambiguous int behaves like an integer type in bounds, but an expression that would overflow (in either direction) yields INT_MIN. Note that, contrary to Francisco's hypothesis, it does *not* act like a float type: expressions like 2147483645 + 1 do have accuracy to the nearest integer. Also, "1 / 2" equals zero even if converted to float. Of course the rules are different for sm6; I haven't checked those yet. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/674#note_62441
On Fri Feb 23 19:36:48 2024 +0000, Francisco Casas wrote:
Please disregard my hypothesis B. I tried that and it didn't work. In fact you even mentioned: ``` % Constant folding is done with uint semantics before converting to float; % so constant folding seems to happen before converting integer operations to floats ``` but maybe there is a lot more going on, for instance, notice this shader: https://shader-playground.timjones.io/df675e96acb6d16bb77dc0ebdf69dfb2 Constant folding is even preempting catastrophic cancellation. Okay, I think that what happens is that float constants use double arithmetic! And this is also true for SM4.
This test can be added, albeit I am not sure if in `sm1-const-folding.shader_test`. ``` % Float constants use double arithmetic on all shader models. [pixel shader] float4 main() : SV_TARGET { float x = (200000000000.0f + 1.0f) - 200000000000.0f ; return x; } [test] draw quad probe all rgba (1.0, 1.0, 1.0, 1.0) ``` -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/674#note_62445
On Fri Feb 23 20:24:27 2024 +0000, Zebediah Figura wrote:
I did some additional testing, and it's more complicated than that. I think I alluded to this elsewhere, but the native compiler has a concept of an "ambiguous" integer type, which is neither int nor uint. You can tell this by doing something like ``` float func(int x) { return 1; } float func(uint x) { return 2; } float4 main() : SV_TARGET { return func(1); } ``` Ambiguous ints are automatically promoted when in an expression with other types, and can of course be implicitly cast, but an expression comprised only of ambiguous ints is still ambiguous. Different integer constants have different types: uint: 1u, 0x1, 01, 0x0, 4294967295u, 4294967296u, 4294967296lu int: 3000000000, -3000000000, 2147483648, -2147483648 ambiguous: 1, 0, 00, 000, -0, -1, 2147483647, -2147483647, 4294967296l, 1 + 2, 1 / 2 empty: 4294967296 Note "empty": that last token lexes as if it were a space. Fun! uint adds with overflow, as you'd expect. Int and ambiguous int behaves like an integer type in bounds, but an expression that would overflow (in either direction) yields INT_MIN. Note that, contrary to Francisco's hypothesis, it does *not* act like a float type: expressions like 2147483645 + 1 do have accuracy to the nearest integer. Also, "1 / 2" equals zero even if converted to float. Of course the rules are different for sm6; I haven't checked those yet. What if the ambiguous int type is actually an ambiguous **number** type -- meaning that it also includes floats and doubles -- and it is represented internally as double? That would explain why there is no loss of accuracy for that value.
Also, "1 / 2" equals zero even if converted to float.
Maybe there is an exception and `/` for ambiguous numbers is always integer division, to be consistent with C. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/674#note_62448
What if the ambiguous int type is actually an ambiguous **number** type -- meaning that it also includes floats and doubles -- and it is represented internally as double? That would explain why there is no loss of accuracy for that value.
That seems unlikely if overflowing bounds always yields INT_MIN. E.g. "return (2147483647 + 1) - 1;" yields INT_MIN. Note also that floats have their own "ambiguous" type. 1.0f is float; 1.0h is half; 1.0 is ambiguous. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/674#note_62449
On Fri Feb 23 20:38:59 2024 +0000, Zebediah Figura wrote:
What if the ambiguous int type is actually an ambiguous **number** type -- meaning that it also includes floats and doubles -- and it is represented internally as double? That would explain why there is no loss of accuracy for that value. That seems unlikely if overflowing bounds always yields INT_MIN. E.g. "return (2147483647 + 1) - 1;" yields INT_MIN. Note also that floats have their own "ambiguous" type. 1.0f is float; 1.0h is half; 1.0 is ambiguous. Similarly (3 / 2) * 3 yields 3, not 4. I think that pretty clearly proves it's converted to integer, at the very least after every step.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/674#note_62450
On Fri Feb 23 20:41:00 2024 +0000, Zebediah Figura wrote:
Similarly (3 / 2) * 3 yields 3, not 4. I think that pretty clearly proves it's converted to integer, at the very least after every step. Zeb, thanks for looking deeper into this. Personally I do not think there is anything urgent for us to do here and I do not plan to spend much more time on this issue, so I would like to know whether you want to upstream yourself some tests which better document your interpretation; if not I'll keep working on this MR (because I think the tests here are valuable, even if likely incomplete), but without necessarily investigating every possible edge case.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/674#note_62629
On Mon Feb 26 10:37:49 2024 +0000, Giovanni Mascellani wrote:
Zeb, thanks for looking deeper into this. Personally I do not think there is anything urgent for us to do here and I do not plan to spend much more time on this issue, so I would like to know whether you want to upstream yourself some tests which better document your interpretation; if not I'll keep working on this MR (because I think the tests here are valuable, even if likely incomplete), but without necessarily investigating every possible edge case. Sorry for the double comment, but the offer to take ownership of this issue is open for Francisco too, of course, if you have bandwidth/curiosity for that.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/674#note_62630
This merge request was approved by Henri Verbeet. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/674
On Mon Feb 26 10:39:09 2024 +0000, Giovanni Mascellani wrote:
Sorry for the double comment, but the offer to take ownership of this issue is open for Francisco too, of course, if you have bandwidth/curiosity for that. Testing and implementing weird behaviour like this is my idea of a good time, so yeah, I wouldn't mind taking ownership.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/674#note_62689
participants (5)
-
Francisco Casas (＠fcasas) -
Giovanni Mascellani
-
Giovanni Mascellani (＠giomasce)
-
Henri Verbeet (＠hverbeet)
-
Zebediah Figura (＠zfigura)