brw: Fix mov cmod propagation when there's int signedness mismatch

If there's difference between scan_inst dest type and inst src type we
should be more careful, because difference in signedness can cause
incorrect results after the propagation.

Updated ror-default.trace hash, as the change fixes misrendering there.

Fixes: b23432c5 ("intel/fs: Fix a cmod prop bug when the source type of a mov doesn't match the dest type of scan_inst")
Signed-off-by: Sviatoslav Peleshko <sviatoslav.peleshko@globallogic.com>
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/30998>
(cherry picked from commit fa51595c7f)

.pick_status.json

@@ -2884,7 +2884,7 @@
         "description": "brw: Fix mov cmod propagation when there's int signedness mismatch",
         "nominated": true,
         "nomination_type": 1,
-        "resolution": 0,
+        "resolution": 1,
         "main_sha": null,
         "because_sha": "b23432c540b1274904d80b90eb67d1798b11d2c8",
         "notes": null

src/intel/ci/traces-iris.yml

@@ -395,17 +395,17 @@ traces:
       checksum: 757e0c9a37ecfc5a2efb10505f98ad95
   ror/ror-default.trace:
     gl-intel-apl:
-      checksum: e604a0d1ecea40e53431e0aa5e6c8276
+      checksum: dde3a4118bca61afdb5fcd94c30e61cd
     gl-intel-glk:
-      checksum: e604a0d1ecea40e53431e0aa5e6c8276
+      checksum: dde3a4118bca61afdb5fcd94c30e61cd
     gl-intel-amly:
-      checksum: e604a0d1ecea40e53431e0aa5e6c8276
+      checksum: dde3a4118bca61afdb5fcd94c30e61cd
     gl-intel-kbl:
-      checksum: e604a0d1ecea40e53431e0aa5e6c8276
+      checksum: dde3a4118bca61afdb5fcd94c30e61cd
     gl-intel-whl:
-      checksum: e604a0d1ecea40e53431e0aa5e6c8276
+      checksum: dde3a4118bca61afdb5fcd94c30e61cd
     gl-intel-cml:
-      checksum: e604a0d1ecea40e53431e0aa5e6c8276
+      checksum: dde3a4118bca61afdb5fcd94c30e61cd
   valve/counterstrike-source-v2.trace:
     gl-intel-apl:
       checksum: 838ed7482efd5e9d7bde2b67e62314c4

src/intel/compiler/brw_fs_cmod_propagation.cpp

@@ -350,6 +350,10 @@ opt_cmod_propagation_local(const intel_device_info *devinfo, bblock_t *block)
                    * - The source of the MOV instruction must be integer with
                    *   the same size.
                    *
+                   * - If the conditional modifier is neither Z nor NZ, then
+                   *   the source of the MOV instruction has to have same
+                   *   signedness.
+                   *
                    * - If the conditional modifier is Z or NZ, then the
                    *   destination type of inst must either be floating point
                    *   (of any size) or integer with a size at least as large
@@ -364,6 +368,12 @@ opt_cmod_propagation_local(const intel_device_info *devinfo, bblock_t *block)
                       brw_type_size_bits(scan_inst->dst.type) != brw_type_size_bits(inst->src[0].type))
                      break;
 
+                  if (inst->conditional_mod != BRW_CONDITIONAL_Z &&
+                      inst->conditional_mod != BRW_CONDITIONAL_NZ &&
+                      brw_type_is_uint(inst->src[0].type) !=
+                      brw_type_is_uint(scan_inst->dst.type))
+                     break;
+
                   if (brw_type_is_int(inst->dst.type)) {
                      if (brw_type_size_bits(inst->dst.type) <
                          brw_type_size_bits(scan_inst->dst.type))

src/intel/compiler/test_fs_cmod_propagation.cpp

@@ -1542,6 +1542,49 @@ TEST_F(cmod_propagation_test, ior_f2i_nz)
    EXPECT_EQ(BRW_CONDITIONAL_NZ, instruction(block0, 1)->conditional_mod);
 }
 
+TEST_F(cmod_propagation_test, uand_b2f_g)
+{
+   brw_reg dest = bld.vgrf(BRW_TYPE_UD);
+   brw_reg src0 = bld.vgrf(BRW_TYPE_UD);
+   brw_reg src1 = bld.vgrf(BRW_TYPE_UD);
+
+   bld.AND(dest, src0, src1);
+   bld.MOV(bld.null_reg_f(), negate(retype(dest, BRW_TYPE_D)))
+   ->conditional_mod = BRW_CONDITIONAL_G;
+
+   /* = Before =
+    * 0: and(8)           dest:UD  src0:UD  src1:UD
+    * 1: mov.g(8)         null:F  -dest:D
+    *
+    * = After =
+    * No changes.
+    *
+    * If src0 and src1 are 0xffffffff, then dest:D will be interpreted as -1,
+    * and -dest:D will be 1, which is > 0.
+    * If the cmod was propagated (and.l(8) dest:UD  src0:UD  src1:UD),
+    * dest:UD can never be < 0.
+    *
+    */
+   v->calculate_cfg();
+   bblock_t *block0 = v->cfg->blocks[0];
+
+   EXPECT_EQ(0, block0->start_ip);
+   EXPECT_EQ(1, block0->end_ip);
+
+   EXPECT_FALSE(cmod_propagation(v));
+   EXPECT_EQ(0, block0->start_ip);
+
+   EXPECT_EQ(BRW_OPCODE_AND, instruction(block0, 0)->opcode);
+   EXPECT_EQ(BRW_CONDITIONAL_NONE, instruction(block0, 0)->conditional_mod);
+
+   /* This is ASSERT_EQ because if end_ip is 0, the instruction(block0, 1)
+    * calls will not work properly, and the test will give weird results.
+    */
+   ASSERT_EQ(1, block0->end_ip);
+   EXPECT_EQ(BRW_OPCODE_MOV, instruction(block0, 1)->opcode);
+   EXPECT_EQ(BRW_CONDITIONAL_G, instruction(block0, 1)->conditional_mod);
+   EXPECT_TRUE(instruction(block0, 1)->src[0].negate);
+}
 
 void
 cmod_propagation_test::test_mov_prop(enum brw_conditional_mod cmod,