diff options
author | Ian Romanick <ian.d.romanick@intel.com> | 2021-01-12 12:23:16 -0800 |
---|---|---|
committer | Dylan Baker <dylan.c.baker@intel.com> | 2021-02-23 13:51:00 -0800 |
commit | 49f6978adcffbe5d45284f7d111d4dc0fdcc8681 (patch) | |
tree | 36a47b404c76be97d72fface8bc2b46fdd7ca947 | |
parent | f656ef672d3006e59b6168903ff5d6d85aebaec9 (diff) |
nir/algebraic: Fix some min/max of b2f replacements
fmin(-A, -B) is -fmax(A, B), and fmax(-A, -B) is -fmin(A, B). Therefore
the logic joining A and B should toggle between ior and iand for the
negated versions.
At the very least, a shader from Euro Truck Simulator 2 in shader-db is
affected by this. The KIL instruction in the (ARB assembly) shader ends
up with the wrong logic. This is _probably_ the source of
https://gitlab.freedesktop.org/mesa/mesa/-/issues/1346.
That said, the issue mentions that Mesa 18.0.5 works, but commit
68420d8322c ("nir: Simplify min and max of b2f") was added in 17.3.
Moreover, I was not able to reproduce the error in the ETS2 shader from
shader-db from any Mesa commit near the time the original fd.o bugzilla
was submitted (December 2018). :shrug:
In fact, the current error in that shader starts with 9167324a86b
("nir/algebraic: Mark some logic-joined comparison reductions as
exact"). That's a bit of a red herring as 9167324a86b just sets off a
chain of replacements that eventually leads to the incorrect min/max of
b2f patterns fixed by this commit.
The other affected shaders in the shader-db results are from Cargo
Commander. These are also ARB assembly shaders.
I think any ARB assembly shader that uses the pattern
SLT r0, ...;
...
KIL -r0;
will suffer from issues related to this.
This change fixes the piglit
tests/spec/arb_fragment_program/kil-of-slt.shader_test test added in
https://gitlab.freedesktop.org/mesa/piglit/-/merge_requests/454.
shader-db results:
All Gen6+ platforms had similar result. (Ice Lake shown)
total instructions in shared programs: 20034604 -> 20034486 (<.01%)
instructions in affected programs: 3885 -> 3767 (-3.04%)
helped: 47
HURT: 2
helped stats (abs) min: 2 max: 4 x̄: 2.64 x̃: 2
helped stats (rel) min: 2.33% max: 8.33% x̄: 3.48% x̃: 3.39%
HURT stats (abs) min: 3 max: 3 x̄: 3.00 x̃: 3
HURT stats (rel) min: 13.64% max: 16.67% x̄: 15.15% x̃: 15.15%
95% mean confidence interval for instructions value: -2.83 -1.99
95% mean confidence interval for instructions %-change: -3.84% -1.60%
Instructions are helped.
total cycles in shared programs: 979881379 -> 979879406 (<.01%)
cycles in affected programs: 119873 -> 117900 (-1.65%)
helped: 46
HURT: 3
helped stats (abs) min: 10 max: 756 x̄: 45.41 x̃: 26
helped stats (rel) min: 0.53% max: 19.72% x̄: 1.67% x̃: 1.26%
HURT stats (abs) min: 28 max: 56 x̄: 38.67 x̃: 32
HURT stats (rel) min: 1.44% max: 3.54% x̄: 2.75% x̃: 3.27%
95% mean confidence interval for cycles value: -70.83 -9.70
95% mean confidence interval for cycles %-change: -2.23% -0.57%
Cycles are helped.
Iron Lake and GM45 had similar results. (Iron Lake shown)
total instructions in shared programs: 8115098 -> 8115076 (<.01%)
instructions in affected programs: 2592 -> 2570 (-0.85%)
helped: 32
HURT: 2
helped stats (abs) min: 1 max: 1 x̄: 1.00 x̃: 1
helped stats (rel) min: 0.88% max: 2.70% x̄: 1.35% x̃: 1.31%
HURT stats (abs) min: 5 max: 5 x̄: 5.00 x̃: 5
HURT stats (rel) min: 17.24% max: 18.52% x̄: 17.88% x̃: 17.88%
95% mean confidence interval for instructions value: -1.15 -0.15
95% mean confidence interval for instructions %-change: -1.83% 1.39%
Inconclusive result (%-change mean confidence interval includes 0).
total cycles in shared programs: 238189718 -> 238189802 (<.01%)
cycles in affected programs: 75076 -> 75160 (0.11%)
helped: 3
HURT: 31
helped stats (abs) min: 2 max: 130 x̄: 44.67 x̃: 2
helped stats (rel) min: 0.18% max: 5.70% x̄: 2.02% x̃: 0.19%
HURT stats (abs) min: 2 max: 70 x̄: 7.03 x̃: 4
HURT stats (rel) min: 0.07% max: 6.41% x̄: 0.53% x̃: 0.15%
95% mean confidence interval for cycles value: -7.27 12.21
95% mean confidence interval for cycles %-change: -0.33% 0.94%
Inconclusive result (value mean confidence interval includes 0).
No fossil-db changes on any Intel platform.
Fixes: 68420d8322c ("nir: Simplify min and max of b2f")
Closes: #1346
Reviewed-by: Matt Turner <mattst88@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/9122>
(cherry picked from commit 7e127c1fca6bd934469f4803dde54842fbd100da)
-rw-r--r-- | .pick_status.json | 2 | ||||
-rw-r--r-- | src/compiler/nir/nir_opt_algebraic.py | 5 |
2 files changed, 4 insertions, 3 deletions
diff --git a/.pick_status.json b/.pick_status.json index 53dfe149dea..266331252e2 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -1336,7 +1336,7 @@ "description": "nir/algebraic: Fix some min/max of b2f replacements", "nominated": true, "nomination_type": 1, - "resolution": 0, + "resolution": 1, "master_sha": null, "because_sha": "68420d8322c2b304a0b15f94b43dec19f082dfae" }, diff --git a/src/compiler/nir/nir_opt_algebraic.py b/src/compiler/nir/nir_opt_algebraic.py index fbd35850cca..52e7f221413 100644 --- a/src/compiler/nir/nir_opt_algebraic.py +++ b/src/compiler/nir/nir_opt_algebraic.py @@ -481,10 +481,11 @@ optimizations.extend([ # a != fsat(a) (('ior', ('flt', a, 0.0), ('flt', 1.0, a)), ('fneu', a, ('fsat', a)), '!options->lower_fsat'), + # Note: fmin(-a, -b) == -fmax(a, b) (('fmax', ('b2f(is_used_once)', 'a@1'), ('b2f', 'b@1')), ('b2f', ('ior', a, b))), - (('fmax', ('fneg(is_used_once)', ('b2f(is_used_once)', 'a@1')), ('fneg', ('b2f', 'b@1'))), ('fneg', ('b2f', ('ior', a, b)))), + (('fmax', ('fneg(is_used_once)', ('b2f(is_used_once)', 'a@1')), ('fneg', ('b2f', 'b@1'))), ('fneg', ('b2f', ('iand', a, b)))), (('fmin', ('b2f(is_used_once)', 'a@1'), ('b2f', 'b@1')), ('b2f', ('iand', a, b))), - (('fmin', ('fneg(is_used_once)', ('b2f(is_used_once)', 'a@1')), ('fneg', ('b2f', 'b@1'))), ('fneg', ('b2f', ('iand', a, b)))), + (('fmin', ('fneg(is_used_once)', ('b2f(is_used_once)', 'a@1')), ('fneg', ('b2f', 'b@1'))), ('fneg', ('b2f', ('ior', a, b)))), # fmin(b2f(a), b) # bcsel(a, fmin(b2f(a), b), fmin(b2f(a), b)) |