nir: Add Midgard-specific fsin/fcos ops

NIR has a fsin instruction that takes an argument in radians. Midgard instead
has an fsinpi argument that takes an argument in multiples of pi. So, we had a
NIR pass that would change fsin(x) to fsin(x / pi) and then map fsin to fsinpi
in the backend.

But that's invalid! In NIR, the opcode fsin is well-defined. fsin(x) means
something very different than fsin(x / pi). They won't usually be equal. The
transform fsin(x) -> fsin(x / pi) is fundamentally unsound.

It did work before, by accident. Most NIR passes don't care about the semantics
of ALU instructions.  fsin(x) and fsin(x / pi) are both well-defined but
fundamentally different NIR shaders. So while rewriting is wrong -- the NIR we
get out is not equivalent to the NIR we put in, and the Midgard ops we generate
are not equivalent to the NIR -- but if we don't run any passes that care about
the definition of fsin the two wrongs will cancel out to make a right.

However, some NIR passes do care about the definitions of ALU instructions,
instead of treating them as named black boxes. In particular, constant folding
(nir_opt_constant_fold) evaluates ALU instructions when their inputs are
constants, according to the definition in nir_opcodes.py. So our little charade
will only work if we don't call nir_opt_constant_fold, or if all the fsin
instructions have non-constant inputs. At the beginning of this series, that is
the case. With the later scalarization change, that's no longer the case, and
the unsoundness translates to real failing tests rather than a quibble of NIR's
semantics.

To mitigate, we define a new NIR opcode with the semantics we want and translate
fsin(x) = fsin_mdg(x / pi), where that equivalence does hold mathematically. So
the new translation is sound and doesn't rely on lucky pass ordering.

This matches the approach already used for AMD and AGX, which have fsin_amd and
fsin_agx opcodes respectively.

Signed-off-by: Alyssa Rosenzweig <alyssa@collabora.com>
Reviewed-by: Italo Nicola <italonicola@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/19350>

This commit is contained in:

Alyssa Rosenzweig

2023-01-13 12:29:14 -05:00

committed by

Marge Bot

parent e86c7ac9f4

commit a49ba0f1ae

1 changed files with 5 additions and 0 deletions

									
										src/compiler/nir/nir_opcodes.py
									
		+5
		
												View File
												
				@@ -1263,6 +1263,11 @@ unop_horiz("cube_r600", 4, tfloat32, 3, tfloat32, """

				unop("fsin_amd", tfloat, "sinf(6.2831853 * src0)")

				unop("fcos_amd", tfloat, "cosf(6.2831853 * src0)")

				# Midgard specific sin and cos

				# These expect their inputs to be divided by pi.

				unop("fsin_mdg", tfloat, "sinf(3.141592653589793 * src0)")

				unop("fcos_mdg", tfloat, "cosf(3.141592653589793 * src0)")

				# AGX specific sin with input expressed in quadrants. Used in the lowering for

				# fsin/fcos. This corresponds to a sequence of 3 ALU ops in the backend (where

				# the angle is further decomposed by quadrant, sinc is computed, and the angle