From d54aa28b970ab09302cba67d5502cb0b4b702a79 Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Thu, 13 Jul 2023 07:13:34 -0400 Subject: [PATCH] nir/legacy: Fix handling of fsat(fabs) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Pavel Ondračka Reviewed-by: Faith Ekstrand Part-of: --- src/compiler/nir/nir_legacy.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/compiler/nir/nir_legacy.c b/src/compiler/nir/nir_legacy.c index fe477356fa7..25e11e9eb9b 100644 --- a/src/compiler/nir/nir_legacy.c +++ b/src/compiler/nir/nir_legacy.c @@ -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)