Mesa (main): intel/fs: Remove type-based restriction for cmod propagation to saturated operations

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Mon Aug 30 21:11:44 UTC 2021


Module: Mesa
Branch: main
Commit: a8d0c0af86c62fdf51dc06e00c9ace5d864c096c
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=a8d0c0af86c62fdf51dc06e00c9ace5d864c096c

Author: Ian Romanick <ian.d.romanick at intel.com>
Date:   Fri Jul 16 18:16:59 2021 -0700

intel/fs: Remove type-based restriction for cmod propagation to saturated operations

Previously, we misunderstood how conditional modifiers and saturate
interacted.  We thought the condition was evaulated before the saturate
was applied.  For the floating point cases, we went to some heroics to
modify the condition to maintain the same results.  For integer cases,
it was not clear that this could even work.  We had no use-cases and no
tests-cases, so we just disallowed everything.

Now we understand that the condition is evaluated after the saturate.
Earlier commits in this series removed the various floating point
heroics.  It is easier to just delete the code that prevents some cases
that just work.

Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12045>

---

 src/intel/compiler/brw_fs_cmod_propagation.cpp  | 18 +-----
 src/intel/compiler/test_fs_cmod_propagation.cpp | 78 +++++++++++++++++--------
 2 files changed, 56 insertions(+), 40 deletions(-)

diff --git a/src/intel/compiler/brw_fs_cmod_propagation.cpp b/src/intel/compiler/brw_fs_cmod_propagation.cpp
index 4bc76caf1ed..afbc34eef23 100644
--- a/src/intel/compiler/brw_fs_cmod_propagation.cpp
+++ b/src/intel/compiler/brw_fs_cmod_propagation.cpp
@@ -512,23 +512,9 @@ opt_cmod_propagation_local(const intel_device_info *devinfo, bblock_t *block)
              *      result is zero after all the clamping, normalizing,
              *      or format conversion logic.
              *
-             * Conditional modifiers can be applied to floating point
-             * calculations without restriction.  Some cases (e.g., .L) are
-             * nonsensical, but those should have been eliminated by the NIR
-             * optimizer.
+             * For this reason, no additional restrictions are necessary on
+             * instructions with saturate.
              */
-            if (scan_inst->saturate) {
-               if (scan_inst->dst.type != BRW_REGISTER_TYPE_F)
-                  break;
-
-               assert(inst->opcode == BRW_OPCODE_MOV ||
-                      inst->opcode == BRW_OPCODE_CMP);
-
-               /* inst->src[1].is_zero() was tested before, but be safe
-                * against possible future changes in this code.
-                */
-               assert(inst->opcode != BRW_OPCODE_CMP || inst->src[1].is_zero());
-            }
 
             /* Otherwise, try propagating the conditional. */
             if (scan_inst->can_do_cmod() &&
diff --git a/src/intel/compiler/test_fs_cmod_propagation.cpp b/src/intel/compiler/test_fs_cmod_propagation.cpp
index e19d8bf632b..6d5851d6448 100644
--- a/src/intel/compiler/test_fs_cmod_propagation.cpp
+++ b/src/intel/compiler/test_fs_cmod_propagation.cpp
@@ -2407,11 +2407,26 @@ TEST_F(cmod_propagation_test, int_saturate_nz_cmp)
     * 1: cmp.nz.f0(8)  null  dest  0
     *
     * = After =
-    * No change.
+    * 0: add.sat.nz.f0(8)    dest  src0  src1
     */
    test_saturate_prop(BRW_CONDITIONAL_NZ, BRW_OPCODE_CMP,
                       BRW_REGISTER_TYPE_D, BRW_REGISTER_TYPE_D,
-                      false);
+                      true);
+}
+
+TEST_F(cmod_propagation_test, uint_saturate_nz_cmp)
+{
+   /* = Before =
+    *
+    * 0: add.sat(8)    dest:UD  src0:UD  src1:UD
+    * 1: cmp.nz.f0(8)  null:D   dest:D   0
+    *
+    * = After =
+    * 0: add.sat.nz.f0(8)    dest:UD  src0:UD  src1:UD
+    */
+   test_saturate_prop(BRW_CONDITIONAL_NZ, BRW_OPCODE_CMP,
+                      BRW_REGISTER_TYPE_UD, BRW_REGISTER_TYPE_D,
+                      true);
 }
 
 TEST_F(cmod_propagation_test, int_saturate_nz_mov)
@@ -2422,11 +2437,11 @@ TEST_F(cmod_propagation_test, int_saturate_nz_mov)
     * 1: mov.nz.f0(8)  null  dest
     *
     * = After =
-    * No change.
+    * 0: add.sat.nz.f0(8)    dest  src0  src1
     */
    test_saturate_prop(BRW_CONDITIONAL_NZ, BRW_OPCODE_MOV,
                       BRW_REGISTER_TYPE_D, BRW_REGISTER_TYPE_D,
-                      false);
+                      true);
 }
 
 TEST_F(cmod_propagation_test, int_saturate_z_cmp)
@@ -2437,11 +2452,26 @@ TEST_F(cmod_propagation_test, int_saturate_z_cmp)
     * 1: cmp.z.f0(8)   null  dest  0
     *
     * = After =
-    * No change.
+    * 0: add.sat.z.f0(8)    dest  src0  src1
     */
    test_saturate_prop(BRW_CONDITIONAL_Z, BRW_OPCODE_CMP,
                       BRW_REGISTER_TYPE_D, BRW_REGISTER_TYPE_D,
-                      false);
+                      true);
+}
+
+TEST_F(cmod_propagation_test, uint_saturate_z_cmp)
+{
+   /* = Before =
+    *
+    * 0: add.sat(8)   dest:UD  src0:UD  src1:UD
+    * 1: cmp.z.f0(8)  null:D   dest:D   0
+    *
+    * = After =
+    * 0: add.sat.z.f0(8)    dest:UD  src0:UD  src1:UD
+    */
+   test_saturate_prop(BRW_CONDITIONAL_Z, BRW_OPCODE_CMP,
+                      BRW_REGISTER_TYPE_UD, BRW_REGISTER_TYPE_D,
+                      true);
 }
 
 TEST_F(cmod_propagation_test, int_saturate_z_mov)
@@ -2455,11 +2485,11 @@ TEST_F(cmod_propagation_test, int_saturate_z_mov)
     * 1: mov.z.f0(8)   null  dest
     *
     * = After =
-    * No change.
+    * 0: add.sat.z.f0(8)    dest  src0  src1
     */
    test_saturate_prop(BRW_CONDITIONAL_Z, BRW_OPCODE_MOV,
                       BRW_REGISTER_TYPE_D, BRW_REGISTER_TYPE_D,
-                      false);
+                      true);
 }
 
 TEST_F(cmod_propagation_test, int_saturate_g_cmp)
@@ -2470,11 +2500,11 @@ TEST_F(cmod_propagation_test, int_saturate_g_cmp)
     * 1: cmp.g.f0(8)   null  dest  0
     *
     * = After =
-    * No change.
+    * 0: add.sat.g.f0(8)    dest  src0  src1
     */
    test_saturate_prop(BRW_CONDITIONAL_G, BRW_OPCODE_CMP,
                       BRW_REGISTER_TYPE_D, BRW_REGISTER_TYPE_D,
-                      false);
+                      true);
 }
 
 TEST_F(cmod_propagation_test, int_saturate_g_mov)
@@ -2485,11 +2515,11 @@ TEST_F(cmod_propagation_test, int_saturate_g_mov)
     * 1: mov.g.f0(8)   null  dest
     *
     * = After =
-    * No change.
+    * 0: add.sat.g.f0(8)    dest  src0  src1
     */
    test_saturate_prop(BRW_CONDITIONAL_G, BRW_OPCODE_MOV,
                       BRW_REGISTER_TYPE_D, BRW_REGISTER_TYPE_D,
-                      false);
+                      true);
 }
 
 TEST_F(cmod_propagation_test, int_saturate_le_cmp)
@@ -2500,11 +2530,11 @@ TEST_F(cmod_propagation_test, int_saturate_le_cmp)
     * 1: cmp.le.f0(8)  null  dest  0
     *
     * = After =
-    * No change.
+    * 0: add.sat.le.f0(8)    dest  src0  src1
     */
    test_saturate_prop(BRW_CONDITIONAL_LE, BRW_OPCODE_CMP,
                       BRW_REGISTER_TYPE_D, BRW_REGISTER_TYPE_D,
-                      false);
+                      true);
 }
 
 TEST_F(cmod_propagation_test, int_saturate_le_mov)
@@ -2515,11 +2545,11 @@ TEST_F(cmod_propagation_test, int_saturate_le_mov)
     * 1: mov.le.f0(8)  null  dest
     *
     * = After =
-    * No change.
+    * 0: add.sat.le.f0(8)    dest  src0  src1
     */
    test_saturate_prop(BRW_CONDITIONAL_LE, BRW_OPCODE_MOV,
                       BRW_REGISTER_TYPE_D, BRW_REGISTER_TYPE_D,
-                      false);
+                      true);
 }
 
 TEST_F(cmod_propagation_test, int_saturate_l_cmp)
@@ -2530,11 +2560,11 @@ TEST_F(cmod_propagation_test, int_saturate_l_cmp)
     * 1: cmp.l.f0(8)  null  dest  0
     *
     * = After =
-    * No change
+    * 0: add.sat.l.f0(8)    dest  src0  src1
     */
    test_saturate_prop(BRW_CONDITIONAL_L, BRW_OPCODE_CMP,
                       BRW_REGISTER_TYPE_D, BRW_REGISTER_TYPE_D,
-                      false);
+                      true);
 }
 
 TEST_F(cmod_propagation_test, int_saturate_l_mov)
@@ -2545,11 +2575,11 @@ TEST_F(cmod_propagation_test, int_saturate_l_mov)
     * 1: mov.l.f0(8)  null  dest  0
     *
     * = After =
-    * No change
+    * 0: add.sat.l.f0(8)    dest  src0  src1
     */
    test_saturate_prop(BRW_CONDITIONAL_L, BRW_OPCODE_MOV,
                       BRW_REGISTER_TYPE_D, BRW_REGISTER_TYPE_D,
-                      false);
+                      true);
 }
 
 TEST_F(cmod_propagation_test, int_saturate_ge_cmp)
@@ -2560,11 +2590,11 @@ TEST_F(cmod_propagation_test, int_saturate_ge_cmp)
     * 1: cmp.ge.f0(8)  null  dest  0
     *
     * = After =
-    * No change
+    * 0: add.sat.ge.f0(8)    dest  src0  src1
     */
    test_saturate_prop(BRW_CONDITIONAL_GE, BRW_OPCODE_CMP,
                       BRW_REGISTER_TYPE_D, BRW_REGISTER_TYPE_D,
-                      false);
+                      true);
 }
 
 TEST_F(cmod_propagation_test, int_saturate_ge_mov)
@@ -2575,11 +2605,11 @@ TEST_F(cmod_propagation_test, int_saturate_ge_mov)
     * 1: mov.ge.f0(8)  null  dest
     *
     * = After =
-    * No change
+    * 0: add.sat.ge.f0(8)    dest  src0  src1
     */
    test_saturate_prop(BRW_CONDITIONAL_GE, BRW_OPCODE_MOV,
                       BRW_REGISTER_TYPE_D, BRW_REGISTER_TYPE_D,
-                      false);
+                      true);
 }
 
 TEST_F(cmod_propagation_test, not_to_or)



More information about the mesa-commit mailing list