Mesa (staging/21.3): pan/bi: Avoid *FADD.v2f16 hazard in optimizer

[GitLab Mirror]

Module: Mesa
Branch: staging/21.3
Commit: 78098039f3c9b6a18d9f0cdbda5cf90c4ec412e3
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=78098039f3c9b6a18d9f0cdbda5cf90c4ec412e3

Author: Alyssa Rosenzweig <alyssa at collabora.com>
Date:   Thu Feb 17 19:33:29 2022 -0500

pan/bi: Avoid *FADD.v2f16 hazard in optimizer

This is a very obscure encoding restriction in the Bifrost ISA. Unknown if any
real apps or tests hit this, but we still need to get it right sadly.

Signed-off-by: Alyssa Rosenzweig <alyssa at collabora.com>
Cc: mesa-stable
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/15072>
(cherry picked from commit 8e0eb592d5bbcf00f8bed55cc95013abf77fad12)

---

 .pick_status.json                       |  2 +-
 src/panfrost/bifrost/bi_opt_mod_props.c | 28 +++++++++++++++++++++++++---
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/.pick_status.json b/.pick_status.json
index 21bfac84d72..3a4da06c144 100644
--- a/.pick_status.json
+++ b/.pick_status.json
@@ -490,7 +490,7 @@
         "description": "pan/bi: Avoid *FADD.v2f16 hazard in optimizer",
         "nominated": true,
         "nomination_type": 0,
-        "resolution": 0,
+        "resolution": 1,
         "main_sha": null,
         "because_sha": null
     },
diff --git a/src/panfrost/bifrost/bi_opt_mod_props.c b/src/panfrost/bifrost/bi_opt_mod_props.c
index 44eae29bd62..ce06f9596ad 100644
--- a/src/panfrost/bifrost/bi_opt_mod_props.c
+++ b/src/panfrost/bifrost/bi_opt_mod_props.c
@@ -25,6 +25,18 @@
 #include "compiler.h"
 #include "bi_builder.h"
 
+/*
+ * Due to a Bifrost encoding restriction, some instructions cannot have an abs
+ * modifier on both sources. Check if adding a fabs modifier to a given source
+ * of a binary instruction would cause this restriction to be hit.
+ */
+static bool
+bi_would_impact_abs(unsigned arch, bi_instr *I, bi_index repl, unsigned s)
+{
+        return (arch <= 8) && I->src[1 - s].abs &&
+               bi_is_word_equiv(I->src[1 - s], repl);
+}
+
 static bool
 bi_takes_fabs(unsigned arch, bi_instr *I, bi_index repl, unsigned s)
 {
@@ -32,9 +44,15 @@ bi_takes_fabs(unsigned arch, bi_instr *I, bi_index repl, unsigned s)
         case BI_OPCODE_FCMP_V2F16:
         case BI_OPCODE_FMAX_V2F16:
         case BI_OPCODE_FMIN_V2F16:
-                /* Bifrost encoding restriction: can't have both abs if equal sources */
-                return !(arch <= 8 && I->src[1 - s].abs
-                                   && bi_is_word_equiv(I->src[1 - s], repl));
+                return !bi_would_impact_abs(arch, I, repl, s);
+        case BI_OPCODE_FADD_V2F16:
+                /*
+                 * For FADD.v2f16, the FMA pipe has the abs encoding hazard,
+                 * while the FADD pipe cannot encode a clamp. Either case in
+                 * isolation can be worked around in the scheduler, but both
+                 * together is impossible to encode. Avoid the hazard.
+                 */
+                return !(I->clamp && bi_would_impact_abs(arch, I, repl, s));
         case BI_OPCODE_V2F32_TO_V2F16:
                 /* TODO: Needs both match or lower */
                 return false;
@@ -182,6 +200,10 @@ bi_takes_clamp(bi_instr *I)
         case BI_OPCODE_FMA_RSCALE_V2F16:
         case BI_OPCODE_FADD_RSCALE_F32:
                 return false;
+        case BI_OPCODE_FADD_V2F16:
+                /* Encoding restriction */
+                return !(I->src[0].abs && I->src[1].abs &&
+                         bi_is_word_equiv(I->src[0], I->src[1]));
         default:
                 return bi_opcode_props[I->op].clamp;
         }