[Mesa-dev] [PATCH v3] i965/vec4: Change types as needed to propagate source modifiers using current instruction

[Alejandro Piñeiro]

SEL and MOV instructions, as long as they don't have source modifiers, are
just copying bits around.  So those kind of instruction could be propagated
even if there are type mismatches. This is needed because NIR generates
integer SEL and MOV instructions whenever it doesn't know what else to
generate.

This commit adds support for copy propagation using current instruction
as reference.

v2: include check for saturate, as Jason Ekstrand suggested
v3: check that the dst.type and the src type are the same, in order to
    solve (among others) the following deqp regression with v2:
    dEQP-GLES3.functional.shaders.operator.unary_operator.minus.lowp_uint_vertex
---

All the patches I sent to this series were tested against a full piglit run.
But while I was waiting for the review, I decided to run a full dEQP run, and
I found that v2 regressed on the following tests:
  * dEQP-GLES3.functional.shaders.operator.unary_operator.minus.lowp_uint_vertex
  * dEQP-GLES3.functional.shaders.operator.unary_operator.minus.lowp_uvec2_vertex
  * dEQP-GLES3.functional.shaders.operator.unary_operator.minus.lowp_uvec3_vertex
  * dEQP-GLES3.functional.shaders.operator.unary_operator.minus.lowp_uvec4_vertex
  * dEQP-GLES3.functional.shaders.operator.unary_operator.minus.mediump_uint_vertex
  * dEQP-GLES3.functional.shaders.operator.unary_operator.minus.mediump_uvec2_vertex
  * dEQP-GLES3.functional.shaders.operator.unary_operator.minus.mediump_uvec3_vertex
  * dEQP-GLES3.functional.shaders.operator.unary_operator.minus.mediump_uvec4_vertex
  * dEQP-GLES3.functional.shaders.operator.unary_operator.minus.highp_uint_vertex
  * dEQP-GLES3.functional.shaders.operator.unary_operator.minus.highp_uvec2_vertex
  * dEQP-GLES3.functional.shaders.operator.unary_operator.minus.highp_uvec3_vertex
  * dEQP-GLES3.functional.shaders.operator.unary_operator.minus.highp_uvec4_vertex
  * dEQP-GLES3.functional.shaders.matrix.mul.const.lowp_mat3x2_mat3_vertex

On those tests we found situation like this:
   7: mov vgrf6.0.x:D, -vgrf5.xxxx:D
   8: mov vgrf0.0.x:F, vgrf6.xxxx:UD

That with v2 was propagated as:
   7: mov vgrf6.0.x:D, -vgrf5.xxxx:D
   8: mov vgrf0.0.x:D, -vgrf5.xxxx:D

Causing the regression. It was fixed by checking too that the type of the
destination and the source are the same.

 .../drivers/dri/i965/brw_vec4_copy_propagation.cpp | 30 ++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp b/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
index 5a15eb8..1522eea 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
@@ -249,6 +249,18 @@ try_constant_propagate(const struct brw_device_info *devinfo,
 }
 
 static bool
+can_change_source_types(vec4_instruction *inst)
+{
+   return inst->dst.type == inst->src[0].type &&
+      !inst->src[0].abs && !inst->src[0].negate && !inst->saturate &&
+      (inst->opcode == BRW_OPCODE_MOV ||
+       (inst->opcode == BRW_OPCODE_SEL &&
+        inst->dst.type == inst->src[1].type &&
+        inst->predicate != BRW_PREDICATE_NONE &&
+        !inst->src[1].abs && !inst->src[1].negate));
+}
+
+static bool
 try_copy_propagate(const struct brw_device_info *devinfo,
                    vec4_instruction *inst,
                    int arg, struct copy_entry *entry)
@@ -308,7 +320,9 @@ try_copy_propagate(const struct brw_device_info *devinfo,
         value.swizzle != BRW_SWIZZLE_XYZW) && !inst->can_do_source_mods(devinfo))
       return false;
 
-   if (has_source_modifiers && value.type != inst->src[arg].type)
+   if (has_source_modifiers &&
+       value.type != inst->src[arg].type &&
+       !can_change_source_types(inst))
       return false;
 
    if (has_source_modifiers &&
@@ -362,7 +376,19 @@ try_copy_propagate(const struct brw_device_info *devinfo,
       }
    }
 
-   value.type = inst->src[arg].type;
+   if (has_source_modifiers &&
+       value.type != inst->src[arg].type) {
+      /* We are propagating source modifiers from a MOV with a different
+       * type.  If we got here, then we can just change the source and
+       * destination types of the instruction and keep going.
+       */
+      assert(can_change_source_types(inst));
+      for (int i = 0; i < 3; i++) {
+         inst->src[i].type = value.type;
+      }
+      inst->dst.type = value.type;
+   } else
+      value.type = inst->src[arg].type;
    inst->src[arg] = value;
    return true;
 }
-- 
2.1.4