intel/fs: cmod propagate from MOV with any condition

There were tests related to propagating conditional modifiers from a MOV
to an instruction with a .SAT modifier for a very long time, but they
were #if'ed out.

There are restrictions later in the function that limit the kinds of MOV
instructions that can propagate.  This avoids the dangers of
type-converting MOVs that may generate flags in different ways.

v2: Update the added comment to look more like the existing comment.
That makes the small differences between the two cases more obvious.
Noticed by Marcin.

All Intel platforms had similar results. (Ice Lake shown)
total instructions in shared programs: 19827127 -> 19826924 (<.01%)
instructions in affected programs: 62024 -> 61821 (-0.33%)
helped: 201
HURT: 0
helped stats (abs) min: 1 max: 2 x̄: 1.01 x̃: 1
helped stats (rel) min: 0.13% max: 0.60% x̄: 0.35% x̃: 0.36%
95% mean confidence interval for instructions value: -1.02 -1.00
95% mean confidence interval for instructions %-change: -0.36% -0.34%
Instructions are helped.

total cycles in shared programs: 954655879 -> 954655356 (<.01%)
cycles in affected programs: 1212877 -> 1212354 (-0.04%)
helped: 155
HURT: 6
helped stats (abs) min: 1 max: 6 x̄: 3.65 x̃: 4
helped stats (rel) min: <.01% max: 0.17% x̄: 0.07% x̃: 0.07%
HURT stats (abs)   min: 2 max: 12 x̄: 7.00 x̃: 8
HURT stats (rel)   min: 0.04% max: 0.23% x̄: 0.14% x̃: 0.15%
95% mean confidence interval for cycles value: -3.60 -2.90
95% mean confidence interval for cycles %-change: -0.07% -0.05%
Cycles are helped.

Reviewed-by: Marcin Ślusarz <marcin.slusarz@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12045>

diff --git a/src/intel/compiler/brw_fs_cmod_propagation.cpp b/src/intel/compiler/brw_fs_cmod_propagation.cpp
index 044d814..3303bc8 100644 (file)
--- a/src/intel/compiler/brw_fs_cmod_propagation.cpp
+++ b/src/intel/compiler/brw_fs_cmod_propagation.cpp
@@ -258,10 +258,6 @@ opt_cmod_propagation_local(const intel_device_info *devinfo, bblock_t *block)
             !inst->src[0].negate))
          continue;
 
-      if (inst->opcode == BRW_OPCODE_MOV &&
-          inst->conditional_mod != BRW_CONDITIONAL_NZ)
-         continue;
-
       /* A CMP with a second source of zero can match with anything.  A CMP
        * with a second source that is not zero can only match with an ADD
        * instruction.
@@ -351,16 +347,26 @@ opt_cmod_propagation_local(const intel_device_info *devinfo, bblock_t *block)
                    *   destination type of inst must either be floating point
                    *   (of any size) or integer with a size at least as large
                    *   as the destination of inst.
+                   *
+                   * - If the conditional modifier is neither Z nor NZ, then the
+                   *   destination type of inst must either be floating point
+                   *   (of any size) or integer with a size at least as large
+                   *   as the destination of inst and the same signedness.
                    */
                   if (!brw_reg_type_is_integer(inst->src[0].type) ||
                       type_sz(scan_inst->dst.type) != type_sz(inst->src[0].type))
                      break;
 
-                  assert(inst->conditional_mod == BRW_CONDITIONAL_NZ);
+                  if (brw_reg_type_is_integer(inst->dst.type)) {
+                     if (type_sz(inst->dst.type) < type_sz(scan_inst->dst.type))
+                        break;
 
-                  if (brw_reg_type_is_integer(inst->dst.type) &&
-                      type_sz(inst->dst.type) < type_sz(scan_inst->dst.type))
-                     break;
+                     if (inst->conditional_mod != BRW_CONDITIONAL_Z &&
+                         inst->conditional_mod != BRW_CONDITIONAL_NZ &&
+                         brw_reg_type_is_unsigned_integer(inst->dst.type) !=
+                         brw_reg_type_is_unsigned_integer(scan_inst->dst.type))
+                        break;
+                  }
                }
             } else {
                /* Not safe to use inequality operators if the types are

diff --git a/src/intel/compiler/test_fs_cmod_propagation.cpp b/src/intel/compiler/test_fs_cmod_propagation.cpp
index f3cb4c4..3bff88e 100644 (file)
--- a/src/intel/compiler/test_fs_cmod_propagation.cpp
+++ b/src/intel/compiler/test_fs_cmod_propagation.cpp
@@ -1614,12 +1614,12 @@ TEST_F(cmod_propagation_test, fadd_fmov_z)
     * 1: mov.z(8)        null:F  dest:F
     *
     * = After =
-    * No changes.  cmod propagation bails on every MOV except MOV.NZ.
+    * 0: add.z(8)        dest:F  src0:F  src1:F
     */
    test_mov_prop(BRW_CONDITIONAL_Z,
                  BRW_REGISTER_TYPE_F,
                  BRW_REGISTER_TYPE_F,
-                 false);
+                 true);
 }
 
 TEST_F(cmod_propagation_test, fadd_fmov_l)
@@ -1629,12 +1629,12 @@ TEST_F(cmod_propagation_test, fadd_fmov_l)
     * 1: mov.l(8)        null:F  dest:F
     *
     * = After =
-    * No changes.  cmod propagation bails on every MOV except MOV.NZ.
+    * 0: add.l(8)        dest:F  src0:F  src1:F
     */
    test_mov_prop(BRW_CONDITIONAL_L,
                  BRW_REGISTER_TYPE_F,
                  BRW_REGISTER_TYPE_F,
-                 false);
+                 true);
 }
 
 TEST_F(cmod_propagation_test, fadd_fmov_g)
@@ -1644,12 +1644,12 @@ TEST_F(cmod_propagation_test, fadd_fmov_g)
     * 1: mov.g(8)        null:F  dest:F
     *
     * = After =
-    * No changes.  cmod propagation bails on every MOV except MOV.NZ.
+    * 0: add.g(8)        dest:F  src0:F  src1:F
     */
    test_mov_prop(BRW_CONDITIONAL_G,
                  BRW_REGISTER_TYPE_F,
                  BRW_REGISTER_TYPE_F,
-                 false);
+                 true);
 }
 
 TEST_F(cmod_propagation_test, fadd_fmov_le)
@@ -1659,12 +1659,12 @@ TEST_F(cmod_propagation_test, fadd_fmov_le)
     * 1: mov.le(8)       null:F  dest:F
     *
     * = After =
-    * No changes.  cmod propagation bails on every MOV except MOV.NZ.
+    * 0: add.le(8)        dest:F  src0:F  src1:F
     */
    test_mov_prop(BRW_CONDITIONAL_LE,
                  BRW_REGISTER_TYPE_F,
                  BRW_REGISTER_TYPE_F,
-                 false);
+                 true);
 }
 
 TEST_F(cmod_propagation_test, fadd_fmov_ge)
@@ -1674,12 +1674,12 @@ TEST_F(cmod_propagation_test, fadd_fmov_ge)
     * 1: mov.ge(8)       null:F  dest:F
     *
     * = After =
-    * No changes.  cmod propagation bails on every MOV except MOV.NZ.
+    * 0: add.ge(8)       dest:F  src0:F  src1:F
     */
    test_mov_prop(BRW_CONDITIONAL_GE,
                  BRW_REGISTER_TYPE_F,
                  BRW_REGISTER_TYPE_F,
-                 false);
+                 true);
 }
 
 TEST_F(cmod_propagation_test, iadd_imov_nz)
@@ -1704,12 +1704,12 @@ TEST_F(cmod_propagation_test, iadd_imov_z)
     * 1: mov.z(8)        null:D  dest:D
     *
     * = After =
-    * No changes.  cmod propagation bails on every MOV except MOV.NZ.
+    * 0: add.z(8)        dest:D  src0:D  src1:D
     */
    test_mov_prop(BRW_CONDITIONAL_Z,
                  BRW_REGISTER_TYPE_D,
                  BRW_REGISTER_TYPE_D,
-                 false);
+                 true);
 }
 
 TEST_F(cmod_propagation_test, iadd_imov_l)
@@ -1719,12 +1719,12 @@ TEST_F(cmod_propagation_test, iadd_imov_l)
     * 1: mov.l(8)        null:D  dest:D
     *
     * = After =
-    * No changes.  cmod propagation bails on every MOV except MOV.NZ.
+    * 0: add.l(8)        dest:D  src0:D  src1:D
     */
    test_mov_prop(BRW_CONDITIONAL_L,
                  BRW_REGISTER_TYPE_D,
                  BRW_REGISTER_TYPE_D,
-                 false);
+                 true);
 }
 
 TEST_F(cmod_propagation_test, iadd_imov_g)
@@ -1734,12 +1734,12 @@ TEST_F(cmod_propagation_test, iadd_imov_g)
     * 1: mov.g(8)        null:D  dest:D
     *
     * = After =
-    * No changes.  cmod propagation bails on every MOV except MOV.NZ.
+    * 0: add.g(8)        dest:D  src0:D  src1:D
     */
    test_mov_prop(BRW_CONDITIONAL_G,
                  BRW_REGISTER_TYPE_D,
                  BRW_REGISTER_TYPE_D,
-                 false);
+                 true);
 }
 
 TEST_F(cmod_propagation_test, iadd_imov_le)
@@ -1749,12 +1749,12 @@ TEST_F(cmod_propagation_test, iadd_imov_le)
     * 1: mov.le(8)       null:D  dest:D
     *
     * = After =
-    * No changes.  cmod propagation bails on every MOV except MOV.NZ.
+    * 0: add.le(8)       dest:D  src0:D  src1:D
     */
    test_mov_prop(BRW_CONDITIONAL_LE,
                  BRW_REGISTER_TYPE_D,
                  BRW_REGISTER_TYPE_D,
-                 false);
+                 true);
 }
 
 TEST_F(cmod_propagation_test, iadd_imov_ge)
@@ -1764,12 +1764,12 @@ TEST_F(cmod_propagation_test, iadd_imov_ge)
     * 1: mov.ge(8)       null:D  dest:D
     *
     * = After =
-    * No changes.  cmod propagation bails on every MOV except MOV.NZ.
+    * 0: add.ge(8)       dest:D  src0:D  src1:D
     */
    test_mov_prop(BRW_CONDITIONAL_GE,
                  BRW_REGISTER_TYPE_D,
                  BRW_REGISTER_TYPE_D,
-                 false);
+                 true);
 }
 
 TEST_F(cmod_propagation_test, iadd_umov_nz)
@@ -1794,14 +1794,12 @@ TEST_F(cmod_propagation_test, iadd_umov_z)
     * 1: mov.z(8)        null:UD dest:D
     *
     * = After =
-    * No changes.  cmod propagation bails on every MOV except MOV.NZ.
-    *
-    * In spite of the type conversion, the Z test would be safe to propagate.
+    * 0: add.z(8)        dest:D  src0:D  src1:D
     */
    test_mov_prop(BRW_CONDITIONAL_Z,
                  BRW_REGISTER_TYPE_D,
                  BRW_REGISTER_TYPE_UD,
-                 false);
+                 true);
 }
 
 TEST_F(cmod_propagation_test, iadd_umov_l)
@@ -1811,7 +1809,7 @@ TEST_F(cmod_propagation_test, iadd_umov_l)
     * 1: mov.l(8)        null:UD dest:D
     *
     * = After =
-    * No changes.  cmod propagation bails on every MOV except MOV.NZ.
+    * No changes.
     *
     * Due to the signed-to-usigned type conversion, the conditional modifier
     * cannot be propagated to the ADD without changing at least the
@@ -1834,7 +1832,7 @@ TEST_F(cmod_propagation_test, iadd_umov_g)
     * 1: mov.g(8)        null:UD dest:D
     *
     * = After =
-    * No changes.  cmod propagation bails on every MOV except MOV.NZ.
+    * No changes.
     *
     * In spite of the type conversion, this could be made to work by
     * propagating NZ instead of G to the ADD.
@@ -1852,7 +1850,7 @@ TEST_F(cmod_propagation_test, iadd_umov_le)
     * 1: mov.le(8)       null:UD dest:D
     *
     * = After =
-    * No changes.  cmod propagation bails on every MOV except MOV.NZ.
+    * No changes.
     *
     * In spite of the type conversion, this could be made to work by
     * propagating Z instead of LE to the ADD.
@@ -1870,7 +1868,7 @@ TEST_F(cmod_propagation_test, iadd_umov_ge)
     * 1: mov.ge(8)       null:UD dest:D
     *
     * = After =
-    * No changes.  cmod propagation bails on every MOV except MOV.NZ.
+    * No changes.
     *
     * Due to the signed-to-usigned type conversion, the conditional modifier
     * cannot be propagated to the ADD without changing at least the
@@ -1916,7 +1914,7 @@ TEST_F(cmod_propagation_test, fadd_f2u_z)
     * 1: mov.z(8)        null:UD dest:F
     *
     * = After =
-    * No changes.  cmod propagation bails on every MOV except MOV.NZ.
+    * No changes.
     *
     * The MOV changes the type from float to unsigned integer.  If dest is in
     * the range [-Inf, 1), the conversion will clamp it to zero.  If dest is
@@ -1936,7 +1934,7 @@ TEST_F(cmod_propagation_test, fadd_f2u_l)
     * 1: mov.l(8)        null:UD dest:F
     *
     * = After =
-    * No changes.  cmod propagation bails on every MOV except MOV.NZ.
+    * No changes.
     *
     * The MOV changes the type from float to unsigned integer.  If dest is in
     * the range [-Inf, 1), the conversion will clamp it to zero.  If dest is
@@ -1956,7 +1954,7 @@ TEST_F(cmod_propagation_test, fadd_f2u_g)
     * 1: mov.g(8)        null:UD dest:F
     *
     * = After =
-    * No changes.  cmod propagation bails on every MOV except MOV.NZ.
+    * No changes.
     *
     * The MOV changes the type from float to unsigned integer.  If dest is in
     * the range [-Inf, 1), the conversion will clamp it to zero.  If dest is
@@ -1976,7 +1974,7 @@ TEST_F(cmod_propagation_test, fadd_f2u_le)
     * 1: mov.le(8)       null:UD dest:F
     *
     * = After =
-    * No changes.  cmod propagation bails on every MOV except MOV.NZ.
+    * No changes.
     *
     * The MOV changes the type from float to unsigned integer.  If dest is in
     * the range [-Inf, 1), the conversion will clamp it to zero.  If dest is
@@ -1996,7 +1994,7 @@ TEST_F(cmod_propagation_test, fadd_f2u_ge)
     * 1: mov.ge(8)       null:UD dest:F
     *
     * = After =
-    * No changes.  cmod propagation bails on every MOV except MOV.NZ.
+    * No changes.
     *
     * The MOV changes the type from float to unsigned integer.  If dest is in
     * the range [-Inf, 1), the conversion will clamp it to zero.  If dest is
@@ -2034,7 +2032,7 @@ TEST_F(cmod_propagation_test, fadd_f2i_z)
     * 1: mov.z(8)        null:D  dest:F
     *
     * = After =
-    * No changes.  cmod propagation bails on every MOV except MOV.NZ.
+    * No changes.
     *
     * The MOV changes the type from float to signed integer.  If dest is in
     * the range (-1, 1), the conversion will clamp it to zero.  If dest is
@@ -2054,7 +2052,7 @@ TEST_F(cmod_propagation_test, fadd_f2i_l)
     * 1: mov.l(8)        null:D  dest:F
     *
     * = After =
-    * No changes.  cmod propagation bails on every MOV except MOV.NZ.
+    * No changes.
     *
     * The MOV changes the type from float to signed integer.  If dest is in
     * the range (-1, 1), the conversion will clamp it to zero.  If dest is
@@ -2074,7 +2072,7 @@ TEST_F(cmod_propagation_test, fadd_f2i_g)
     * 1: mov.g(8)        null:D  dest:F
     *
     * = After =
-    * No changes.  cmod propagation bails on every MOV except MOV.NZ.
+    * No changes.
     *
     * The MOV changes the type from float to signed integer.  If dest is in
     * the range (-1, 1), the conversion will clamp it to zero.  If dest is
@@ -2094,7 +2092,7 @@ TEST_F(cmod_propagation_test, fadd_f2i_le)
     * 1: mov.le(8)       null:D  dest:F
     *
     * = After =
-    * No changes.  cmod propagation bails on every MOV except MOV.NZ.
+    * No changes.
     *
     * The MOV changes the type from float to signed integer.  If dest is in
     * the range (-1, 1), the conversion will clamp it to zero.  If dest is
@@ -2114,7 +2112,7 @@ TEST_F(cmod_propagation_test, fadd_f2i_ge)
     * 1: mov.ge(8)       null:D  dest:F
     *
     * = After =
-    * No changes.  cmod propagation bails on every MOV except MOV.NZ.
+    * No changes.
     *
     * The MOV changes the type from float to signed integer.  If dest is in
     * the range (-1, 1), the conversion will clamp it to zero.  If dest is
@@ -2257,18 +2255,10 @@ TEST_F(cmod_propagation_test, float_saturate_z_mov)
     * = After =
     * 0: add.sat.le.f0(8)  dest  src0  src1
     */
-#if 1
-   /* cmod propagation bails on every MOV except MOV.NZ. */
-   test_saturate_prop(BRW_CONDITIONAL_Z, BRW_CONDITIONAL_Z,
-                      BRW_OPCODE_MOV,
-                      BRW_REGISTER_TYPE_F, BRW_REGISTER_TYPE_F,
-                      false);
-#else
    test_saturate_prop(BRW_CONDITIONAL_Z, BRW_CONDITIONAL_LE,
                       BRW_OPCODE_MOV,
                       BRW_REGISTER_TYPE_F, BRW_REGISTER_TYPE_F,
                       true);
-#endif
 }
 
 TEST_F(cmod_propagation_test, float_saturate_g_cmp)
@@ -2303,18 +2293,10 @@ TEST_F(cmod_propagation_test, float_saturate_g_mov)
     * = After =
     * 0: add.sat.g.f0(8)  dest  src0  src1
     */
-#if 1
-   /* cmod propagation bails on every MOV except MOV.NZ. */
-   test_saturate_prop(BRW_CONDITIONAL_G, BRW_CONDITIONAL_G,
-                      BRW_OPCODE_MOV,
-                      BRW_REGISTER_TYPE_F, BRW_REGISTER_TYPE_F,
-                      false);
-#else
    test_saturate_prop(BRW_CONDITIONAL_G, BRW_CONDITIONAL_G,
                       BRW_OPCODE_MOV,
                       BRW_REGISTER_TYPE_F, BRW_REGISTER_TYPE_F,
                       true);
-#endif
 }
 
 TEST_F(cmod_propagation_test, float_saturate_le_cmp)
@@ -2349,18 +2331,10 @@ TEST_F(cmod_propagation_test, float_saturate_le_mov)
     * = After =
     * 0: add.sat.le.f0(8)  dest  src0  src1
     */
-#if 1
-   /* cmod propagation bails on every MOV except MOV.NZ. */
-   test_saturate_prop(BRW_CONDITIONAL_LE, BRW_CONDITIONAL_LE,
-                      BRW_OPCODE_MOV,
-                      BRW_REGISTER_TYPE_F, BRW_REGISTER_TYPE_F,
-                      false);
-#else
    test_saturate_prop(BRW_CONDITIONAL_LE, BRW_CONDITIONAL_LE,
                       BRW_OPCODE_MOV,
                       BRW_REGISTER_TYPE_F, BRW_REGISTER_TYPE_F,
                       true);
-#endif
 }
 
 TEST_F(cmod_propagation_test, float_saturate_l_cmp)