nir/algebraic: Avoid creating new fp64 ops when using softfp64
authorKenneth Graunke <kenneth@whitecape.org>
Tue, 1 Dec 2020 00:14:55 +0000 (16:14 -0800)
committerMarge Bot <eric+marge@anholt.net>
Tue, 1 Dec 2020 06:29:31 +0000 (06:29 +0000)
commit531843cf2e939b764822ef56ba8e034ad417a812
tree9d6f7bd5504b0a2e5a23a5cdaef58a276c8bfe65
parent688dda5e1dd867607fbf837f6a5ebfdeeb8dd66e
nir/algebraic: Avoid creating new fp64 ops when using softfp64

In commit 00b28a50b2c492eee25ef3f75538aabe1e569ff1, 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>
src/compiler/nir/nir_opt_algebraic.py