summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorIan Romanick <ian.d.romanick@intel.com>2021-01-12 12:23:16 -0800
committerDylan Baker <dylan.c.baker@intel.com>2021-02-23 13:51:00 -0800
commit49f6978adcffbe5d45284f7d111d4dc0fdcc8681 (patch)
tree36a47b404c76be97d72fface8bc2b46fdd7ca947
parentf656ef672d3006e59b6168903ff5d6d85aebaec9 (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.json2
-rw-r--r--src/compiler/nir/nir_opt_algebraic.py5
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))