It seems worth the small amount of damage to give an extra cushion of
not having to debug problems later.
Reviewed-by: Rhys Perry <pendingchaos02@gmail.com>
All Intel platforms had similar results. (Tiger Lake shown)
total instructions in shared programs: 21043197 -> 21043359 (<.01%)
instructions in affected programs: 4409 -> 4571 (3.67%)
helped: 0
HURT: 25
HURT stats (abs) min: 1 max: 16 x̄: 6.48 x̃: 5
HURT stats (rel) min: 0.39% max: 15.38% x̄: 4.59% x̃: 4.40%
95% mean confidence interval for instructions value: 4.37 8.59
95% mean confidence interval for instructions %-change: 2.93% 6.26%
Instructions are HURT.
total cycles in shared programs: 856175986 -> 856176921 (<.01%)
cycles in affected programs: 58908 -> 59843 (1.59%)
helped: 0
HURT: 25
HURT stats (abs) min: 7 max: 70 x̄: 37.40 x̃: 38
HURT stats (rel) min: 0.27% max: 5.63% x̄: 1.87% x̃: 1.39%
95% mean confidence interval for cycles value: 31.11 43.69
95% mean confidence interval for cycles %-change: 1.35% 2.39%
Cycles are HURT.
No fossil-db changes on any Intel platform.
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/10012>
When most of these patterns were created, we believed, incorrectly, that
fsat(NaN) was NaN. We have since realized that fsat(NaN) is zero.
Originally, this changed the patterns to use is_a_number. This didn't
help any shaders, so it's easier to just drop the optimizations.
This commit crossed paths with 4c3ad4d065 ("nir/algebraic: mark more
optimization with fsat(NaN) as inexact") and bc123c396a
("nir/algebraic: mark some optimizations with fsat(NaN) as inexact").
Given that these don't impact very many shaders, it seems safer to just
remove them.
As discussed in
https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8716, I tried
modifying these patterns to use !(b cmp a). Unfortunately, on Intel
GPUs, the results were much worse than just removing the patterns
altogether.
Some other related patterns will be addressed in later commits.
There are still a number of patterns that use the identity fsat(1-X) ==
1 - fsat(X). If X is NaN, the former is zero while the latter is 1.0.
I haven't evaluted these patterns yet. If changes are needed in these
patterns, it should be a separate commit anyway.
v2: Replace arrow `=>` with `->` in comments because the `=>` looks a
lot like `<=` comparison. Suggested by Rhys.
Fixes: 92b75c126b ("nir/algebraic: Replace checks that a value is between (or not) [0, 1]")
Fixes: a7f0c57673 ("nir/algebraic: Eliminate useless fsat() on operand of comparison w/value in (0, 1)")
Reviewed-by: Rhys Perry <pendingchaos02@gmail.com>
All Intel hardware had similar results. (Ice Lake shown)
total instructions in shared programs: 20029060 -> 20029670 (<.01%)
instructions in affected programs: 69236 -> 69846 (0.88%)
helped: 0
HURT: 263
HURT stats (abs) min: 1 max: 20 x̄: 2.32 x̃: 1
HURT stats (rel) min: 0.30% max: 11.11% x̄: 1.35% x̃: 0.98%
95% mean confidence interval for instructions value: 1.86 2.78
95% mean confidence interval for instructions %-change: 1.18% 1.52%
Instructions are HURT.
total cycles in shared programs: 979821278 -> 979834425 (<.01%)
cycles in affected programs: 1476848 -> 1489995 (0.89%)
helped: 49
HURT: 204
helped stats (abs) min: 1 max: 812 x̄: 102.31 x̃: 20
helped stats (rel) min: 0.01% max: 21.43% x̄: 2.23% x̃: 0.52%
HURT stats (abs) min: 2 max: 2600 x̄: 89.02 x̃: 16
HURT stats (rel) min: 0.04% max: 27.27% x̄: 1.49% x̃: 0.72%
95% mean confidence interval for cycles value: 13.18 90.75
95% mean confidence interval for cycles %-change: 0.29% 1.25%
Cycles are HURT.
No fossil-db changes.
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/10012>
HLSL doesn't support bitcasting a 64bit integer to a double. DXIL
doesn't have generic pack/unpack instructions, so we lower those to
integer bitwise ops. As a result, NIR generic double pack/unpack would
require our backend to emit a bitcast to get a double, but we want
to match HLSL semantics and emit MakeDouble/SplitDouble.
Adding a dedicated opcode for double pack/unpack allows us to add a
pass to emit that instead, which lets our backend emit the right
instruction to pack and unpack doubles.
Acked-by: Jason Ekstrand <jason@jlekstrand.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/10063>
There are patterns that will re-write the fmin or fmax part into a form
that other patterns will gradually convert to the same ior or iand. For
example,
fmax(b2f(a), b2f(b)) != 0
b2f(a || b) != 0
a || b
No shader-db or fossil-db changes on any Intel platform.
Reviewed-by: Matt Turner <mattst88@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/9122>
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
68420d8322 ("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). 🤷
In fact, the current error in that shader starts with 9167324a86
("nir/algebraic: Mark some logic-joined comparison reductions as
exact"). That's a bit of a red herring as 9167324a86 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: 68420d8322 ("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>
The base mask previously used was 0xffffffff. This is not correct (but
should still work) for 16-bit and 8-bit values, but it means the high
32-bits of 64-bit values will get chopped off.
Instead of just restricting the pattern to 32-bits (as was done before
00b28a50b2), this extends the optimization in two ways:
1. Make it correct for other bit sizes.
2. Make it work for arbitrary shift counts.
This has the added benefit of reducing the number of patterns actually
added (7 previously, 4 now).
The "Reassociate for improved CSE" part is just reverted to its
pre-00b28a50b2c behavior. I doubt that pattern is likely to have much
impact outside 32-bits.
This change fixes the piglit tests
tests/spec/arb_gpu_shader_int64/fs-shl-of-shr-int64.shader_test and
tests/spec/arb_gpu_shader_int64/fs-iand-of-iadd-int64.shader_test.
All of the shaders helped in shader-db are vertex shaders on platforms
with vector-oriented vertex processing. The shaders contain ((x >> 16)
<< 16). These platforms set lower_extract_word, so the optimization
that transforms (x >> 16) to extract_u16 doesn't trigger. With only ~60
shaders involved, I didn't bother trying to add extract_XYZ versions of
these patterns to try to get those cases.
Fixes: 00b28a50b2 ("nir/algebraic: trivially enable existing 32-bit patterns for all bit sizes")
Reviewed-by: Rhys Perry <pendingchaos02@gmail.com>
Haswell and earlier Intel GPUs had simlar results. (Haswell shown)
total instructions in shared programs: 16397554 -> 16397496 (<.01%)
instructions in affected programs: 7961 -> 7903 (-0.73%)
helped: 58
HURT: 0
helped stats (abs) min: 1 max: 1 x̄: 1.00 x̃: 1
helped stats (rel) min: 0.36% max: 1.89% x̄: 0.99% x̃: 0.78%
95% mean confidence interval for instructions value: -1.00 -1.00
95% mean confidence interval for instructions %-change: -1.13% -0.85%
Instructions are helped.
total cycles in shared programs: 1035483770 -> 1035483504 (<.01%)
cycles in affected programs: 75922 -> 75656 (-0.35%)
helped: 44
HURT: 2
helped stats (abs) min: 2 max: 12 x̄: 6.14 x̃: 2
helped stats (rel) min: 0.05% max: 1.67% x̄: 0.87% x̃: 0.72%
HURT stats (abs) min: 2 max: 2 x̄: 2.00 x̃: 2
HURT stats (rel) min: 0.06% max: 0.06% x̄: 0.06% x̃: 0.06%
95% mean confidence interval for cycles value: -7.28 -4.29
95% mean confidence interval for cycles %-change: -1.03% -0.63%
Cycles are helped.
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8852>
This patch also replaces lower_negate with lower_ineg / lower_fneg.
The fneg semantics have been clarified as of Version 1.5, Revision 1
of the SPIR-V specification, which means that the previous lowering
to fsub is not a viable solution anymore, and is replaced with
lowering to fmul(x, -1.0).
Reviewed-by: Erik Faye-Lund <erik.faye-lund@collabora.com>
Reviewed-by: Eric Anholt <eric@anholt.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6597>
If multiple rules could match, the rule that appears first in the file
is used.
Only Tiger Lake and Ice Lake are affected. Other platforms either have
a LRP instruction or can't run any shaders from shader-db that would
benefit.
v2: Fix issues created when this commit was rebased on top of
3c8934a644 ("nir/algebraic: add flrp patterns for 16 and 64 bits").
Noticed by Caio.
Tiger Lake and Ice Lake had similar results.
total instructions in shared programs: 20908672 -> 20908661 (<.01%)
instructions in affected programs: 419 -> 408 (-2.63%)
helped: 5
HURT: 0
helped stats (abs) min: 1 max: 3 x̄: 2.20 x̃: 3
helped stats (rel) min: 1.85% max: 3.19% x̄: 2.49% x̃: 2.65%
95% mean confidence interval for instructions value: -3.56 -0.84
95% mean confidence interval for instructions %-change: -3.24% -1.73%
Instructions are helped.
total cycles in shared programs: 473513940 -> 473513793 (<.01%)
cycles in affected programs: 7176 -> 7029 (-2.05%)
helped: 12
HURT: 0
helped stats (abs) min: 5 max: 22 x̄: 12.25 x̃: 12
helped stats (rel) min: 0.84% max: 3.24% x̄: 2.09% x̃: 1.80%
95% mean confidence interval for cycles value: -15.43 -9.07
95% mean confidence interval for cycles %-change: -2.57% -1.61%
Cycles are helped.
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6358>
This prevents other transformations from converting them to 'a != 0'.
For example, both of these transformations can do this:
(('~flt', 0.0, ('fabs', a)), ('fne', a, 0.0)),
(('~flt', ('fneg', ('fabs', a)), 0.0), ('fne', a, 0.0)),
Both fsign(fabs(NaN)) and fsign(fneg(fabs(NaN))) should produce zero,
but, since 'NaN != 0.0' is true, cascading these transformations could
cause them to generate 1.0 or -1.0 respecively.
No shader-db or fossil-db changes on any Intel platform.
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6358>
This prevents some fossil-db regressions in "spir-v: Mark floating point
comparisons exact".
v2: Note that the patterns and replacements produce the same value when
isnan(b). Suggested by Caio.
v3: Use C99 isfinite() instead of (obsolete) BSD finite(). Fixes
various Windows builds.
No fossil-db changes on any Inetl platform, Vega, or Polaris10.
All Intel platforms had similar results. (Tiger Lake shown)
total instructions in shared programs: 20908670 -> 20908672 (<.01%)
instructions in affected programs: 69 -> 71 (2.90%)
helped: 0
HURT: 1
total cycles in shared programs: 473515288 -> 473513940 (<.01%)
cycles in affected programs: 4942 -> 3594 (-27.28%)
helped: 2
HURT: 0
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6358>
This also prevents some fossil-db regressions in "spir-v: Mark floating
point comparisons exact".
v2: Mark the fmin / fmax in the replacement exact to prevent other
optimizations from ruining the NaN-clensing property of the fmin / fmax.
Suggested by Rhys. Don't assume that constants are not NaN because some
components of a vector might be NaN while others are numbers. Noticed
by Rhys. This causes ~8 more shaders in Age of Wonders III (dxvk) to
regress on cycles (not instructions) by less than 1% when "spir-v: Mark
floating point comparisons exact" is applied. This difference is too
small to care.
All Intel platforms had similar results. (Tiger Lake shown)
total instructions in shared programs: 20908668 -> 20908670 (<.01%)
instructions in affected programs: 9196 -> 9198 (0.02%)
helped: 10
HURT: 5
helped stats (abs) min: 1 max: 2 x̄: 1.40 x̃: 1
helped stats (rel) min: 0.02% max: 5.41% x̄: 2.20% x̃: 2.16%
HURT stats (abs) min: 2 max: 6 x̄: 3.20 x̃: 3
HURT stats (rel) min: 2.44% max: 16.67% x̄: 9.39% x̃: 12.50%
95% mean confidence interval for instructions value: -1.22 1.49
95% mean confidence interval for instructions %-change: -2.08% 5.41%
Inconclusive result (value mean confidence interval includes 0).
total cycles in shared programs: 473515330 -> 473515288 (<.01%)
cycles in affected programs: 67146 -> 67104 (-0.06%)
helped: 10
HURT: 7
helped stats (abs) min: 1 max: 36 x̄: 15.90 x̃: 17
helped stats (rel) min: 0.01% max: 1.29% x̄: 0.66% x̃: 0.89%
HURT stats (abs) min: 1 max: 48 x̄: 16.71 x̃: 4
HURT stats (rel) min: 0.08% max: 1.94% x̄: 0.87% x̃: 0.19%
95% mean confidence interval for cycles value: -13.88 8.94
95% mean confidence interval for cycles %-change: -0.56% 0.49%
Inconclusive result (value mean confidence interval includes 0).
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6358>
I originally noticed that 3b30814791 ("nir/algebraic: Optimize 1-bit
Booleans") caused this pattern no longer be matched by incorrectly
replacing b@32 with b@1. Making that correct had no effect on
shader-db. When this pattern originally was added, it only affected 4
shaders, so it's not worth the effort to debug further.
This reverts commit f50400cc80.
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6358>
The original comment was a little terse and a little incorrect. The
rearrangements are fine w.r.t. NaN. However, they produce incorrect
results if one operand is +Inf and the other is -Inf.
A later commit, "nir/algebraic: Add some compare-with-zero optimizations
that are exact", will add some more patterns here. It may be reasonable
to squash this commit (forward) into that commit.
v2: Fix some incorrect comparisons operators in the comment (<= vs >=).
Add commentary that subtraction works like addition w.r.t. NaN. Both
noticed / suggested by Caio.
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6358>
This commit only documents the current behavior, even if that behavior
is not the behavior preferred by the relevant specs.
In SPIR-V, there are two flavors of the sign instruction, and each lives
in an extended instruction set. The GLSL.std.450 FSign instruction is
defined as:
Result is 1.0 if x > 0, 0.0 if x = 0, or -1.0 if x < 0.
This also matches the GLSL 4.60 definition.
However, the OpenCL.ExtendedInstructionSet.100 sign instruction is
defined as:
Returns 1.0 if x > 0, -0.0 if x = -0.0, +0.0 if x = +0.0, or -1.0 if
x < 0. Returns 0.0 if x is a NaN.
There are two differences. Each treats -0.0 differently, and each also
treats NaN differently. Specifically, GLSL.std.450 FSign does not
define any specific behavior for NaN.
There has been some discussion in Khronos about the NaN behavior of
GLSL.std.450 FSign. As part of that discussion, I did some research
into how we treat NaN for nir_op_fsign, and this commit just captures
some of those notes.
v2: Document the expected behavior of nir_op_fsign more thoroughly.
Suggested by Rhys. Note that the current implementation of constant
folding does not produce the expected result for NaN. Suggested by
Caio.
Reviewed-by: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com> [v1]
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6358>
In commit 00b28a50b2, Marek extended
a number of optimizations that had been 32-bit specific to work on
other bit-sizes.
Most optimizations preserve the data type across the transformation.
In other words, an optimization which generates e.g. fp64 operations
only does so when the source expression also contains fp64 operations.
These transformations are fine with respect to lowering, because we
will lower away all expressions that would trigger the search portion
of the expression, and so we'd never apply those rules.
However, a few of the rules create new operations that run afoul of
lowering passes. For example,
('bcsel', a, 1.0, 0.0) => ('b2f', a)
where the result is a double would simply be a selection between two
different 64-bit constants. The replacement expression, on the other
hand, involves a nir_op_b2f64 ALU operation. If we're run after
nir_lower_doubles, then it may not be legal to generate such an
expression anymore (at least without running lowering again, which we
don't do today).
Regressions due to this are blocking the 20.3 release, so for now, we
take the easy route and simply disallow those few rules when doing full
softfp64 lowering, which fixes the immediate problem. But it doesn't
solve the long-term problem in an extensible manner.
In the future, we may want to add a `lowered_alu_ops` bitfield to the
NIR shader, and as lowering passes are run, mark them as taboo. Then,
we could have each algebraic transformation track which operations it
creates in the replacement expression. With both of those in place,
nir_replace_instr could compare the transformation's list of ALU ops
against `lowered_alu_ops` and implicitly skip rules that generate
forbidden ALU operations.
Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/3504
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7841>
All these instructions replicate the result of a N-component dot-product
to a vec4. Naming them fdot_replicatedN gives the impression that are
some sort of abstract dot-product that replicates the result to a vecN.
They also deviate from fdph_replicated... which nobody would reasonably
consider naming fdot_replicatedh.
Naming these opcodes fdotN_replicated more closely matches what they
are, and it matches the pattern of fdph_replicated.
I believe that the only reason these opcodes were named this way was
because it simplified the implementation of the binop_reduce function in
nir_opcodes.py. I made some fairly simple changes to that function, and
I think the end result is ok.
The bulk of the changes come from the sed rename:
sed --in-place -e 's/fdot_replicated\([234]\)/fdot\1_replicated/g' \
$(grep -r 'fdot_replicated[234]' src/)
v2: Use a named parameter to binop_reduce instead of using
isinstance(name, str). Suggested by Jason.
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/5725>
Before 8e1b75b330 ("nir/algebraic: optimize iand/ior of (n)eq zero") this
optimization didn't need the use of umax/umin. VC4 HW supports only signed
integer max/min operations.
lower_umin and lower_umax are added to allow enabling previous optimizations
behaviour for this cases.
Fixes: 8e1b75b330 ("nir/algebraic: optimize iand/ior of (n)eq zero")
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Reviewed-by: Daniel Schürmann <daniel@schuermann.dev>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7083>
