nir/legacy: Fix handling of fsat(fabs)
authorAlyssa Rosenzweig <alyssa@rosenzweig.io>
Thu, 13 Jul 2023 11:13:34 +0000 (07:13 -0400)
committerMarge Bot <emma+marge@anholt.net>
Thu, 13 Jul 2023 22:43:36 +0000 (22:43 +0000)
Consider code like:

    32x4  %2 = @load_interpolated_input (%1, %0 (0x0)) (base=0, component=0, dest_type=float32, io location=VARYING_SLOT_VAR0 slots=1 mediump)  // Color
    32x4  %3 = fabs %2
    32x4  %4 = fsat %3
    32x4  %5 = fsin %4

The existing logic would incorrectly tell the backend that both fabs and fsat
could be folded, and then half the shader disappears. Whoops. Fix by stopping
the folding in this case. I choose to do this check in the fsat rather than the
fabs because it's more straightforward (1 source vs N uses) but it's somewhat
arbitrary.

Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
Reviewed-by: Pavel Ondračka <pavel.ondracka@gmail.com>
Reviewed-by: Faith Ekstrand <faith.ekstrand@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/24116>

src/compiler/nir/nir_legacy.c

index fe47735..25e11e9 100644 (file)
@@ -183,6 +183,12 @@ nir_legacy_fsat_folds(nir_alu_instr *fsat)
    if (dest_type != nir_type_float)
       return false;
 
+   /* If we are a saturating a source modifier fsat(fabs(x)), we need to emit
+    * either the fsat or the modifier or else the sequence disappears.
+    */
+   if (generate_alu->op == nir_op_fabs || generate_alu->op == nir_op_fneg)
+      return false;
+
    /* We can't do expansions without a move in the middle */
    unsigned nr_components = nir_dest_num_components(generate_alu->dest.dest);
    if (fsat->dest.dest.ssa.num_components != nr_components)