commit 6a19d8915fe2e07509ab2eeeea268af9728c3b42
author Ian Romanick <ian.d.romanick@intel.com> Mon Mar 10 11:37:50 2025 -0700
committer Marge Bot <emma+marge@anholt.net> Tue Apr 15 23:59:31 2025 +0000
tree 575b581ef1ea6c600dfd5e117975f236eaf47ee6
parent 07dc1d40437ce6a5ac417f0b20146d9512a595a1

brw/algebraic: Don't optimize float SEL.CMOD to MOV

Floating point SEL.CMOD may flush denorms to zero. We don't have enough
information at this point in compilation to know whether or not it is
safe to remove that.

Integer SEL or SEL without a conditional modifier is just a fancy
MOV. Those are always safe to eliminate.

See also 3f782cdd2591.

Fixes: fab92fa1cba ("i965/fs: Optimize SEL with the same sources into a MOV.")

No shader-db changes on any Intel platform.

fossil-db:

All Intel platforms had similar results. (Lunar Lake shown)
Totals:
Instrs: 209903490 -> 209903492 (+0.00%)
Cycle count: 30546025224 -> 30546021980 (-0.00%); split: -0.00%, +0.00%
Max live registers: 65516231 -> 65516235 (+0.00%)

Totals from 2 (0.00% of 706657) affected shaders:
Instrs: 3197 -> 3199 (+0.06%)
Cycle count: 361650 -> 358406 (-0.90%); split: -10.05%, +9.15%
Max live registers: 300 -> 304 (+1.33%)

Reviewed-by: Ivan Briano <ivan.briano@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/34192>

src/intel/compiler/brw_opt_algebraic.cpp

diff --git a/src/intel/compiler/brw_opt_algebraic.cpp b/src/intel/compiler/brw_opt_algebraic.cpp
index ab4005b..db6f2a4 100644
--- a/src/intel/compiler/brw_opt_algebraic.cpp
+++ b/src/intel/compiler/brw_opt_algebraic.cpp
@@ -546,7 +546,16 @@
          }
          break;
       case BRW_OPCODE_SEL:
-         if (inst->src[0].equals(inst->src[1])) {
+         /* Floating point SEL.CMOD may flush denorms to zero. We don't have
+          * enough information at this point in compilation to know whether or
+          * not it is safe to remove that.
+          *
+          * Integer SEL or SEL without a conditional modifier is just a fancy
+          * MOV. Those are always safe to eliminate.
+          */
+         if (inst->src[0].equals(inst->src[1]) &&
+             (!brw_type_is_float(inst->dst.type) ||
+              inst->conditional_mod == BRW_CONDITIONAL_NONE)) {
             inst->opcode = BRW_OPCODE_MOV;
             inst->predicate = BRW_PREDICATE_NONE;
             inst->predicate_inverse = false;

src/intel/compiler/test_opt_algebraic.cpp

diff --git a/src/intel/compiler/test_opt_algebraic.cpp b/src/intel/compiler/test_opt_algebraic.cpp
index fdbd95f..81bc8d6 100644
--- a/src/intel/compiler/test_opt_algebraic.cpp
+++ b/src/intel/compiler/test_opt_algebraic.cpp
@@ -24,3 +24,37 @@
 
    EXPECT_SHADERS_MATCH(bld, exp);
 }
+
+TEST_F(algebraic_test, sel_a_a)
+{
+   brw_builder bld = make_shader(MESA_SHADER_FRAGMENT, 16);
+   brw_builder exp = make_shader(MESA_SHADER_FRAGMENT, 16);
+
+   brw_reg dst0 = vgrf(bld, exp, BRW_TYPE_D);
+   brw_reg src0 = vgrf(bld, exp, BRW_TYPE_D);
+
+   bld.SEL(dst0, src0, src0)
+      ->predicate = BRW_PREDICATE_NORMAL;
+
+   EXPECT_PROGRESS(brw_opt_algebraic, bld);
+
+   exp.MOV(dst0, src0);
+
+   EXPECT_SHADERS_MATCH(bld, exp);
+}
+
+TEST_F(algebraic_test, fmax_a_a)
+{
+   brw_builder bld = make_shader(MESA_SHADER_FRAGMENT, 16);
+
+   brw_reg dst0 = vgrf(bld, BRW_TYPE_F);
+   brw_reg src0 = vgrf(bld, BRW_TYPE_F);
+
+   bld.emit_minmax(dst0, src0, src0, BRW_CONDITIONAL_GE);
+
+   /* SEL.GE may flush denorms to zero. We don't have enough information at
+    * this point in compilation to know whether or not it is safe to remove
+    * that.
+    */
+   EXPECT_NO_PROGRESS(brw_opt_algebraic, bld);
+}