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

[Alejandro Piñeiro]

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 MOV instructions whenever it doesn't know what
else to generate.

This commit adds support for copy propagation using previous (from)
instruction as reference.

Shader-db results for vec4 programs on Haswell:
total instructions in shared programs: 1683959 -> 1669037 (-0.89%)
instructions in affected programs:     746238 -> 731316 (-2.00%)
helped:                                6264
HURT:                                  30

v2: using 'arg' index to get the from inst was wrong, as pointed
    by Jason Ekstrand
v3: rebased against last change on the previous patch of the series
v4: don't need to track instructions on struct copy_entry, as we
    only set the source on a direct copy, as pointed by Jason
---
 .../drivers/dri/i965/brw_vec4_copy_propagation.cpp | 36 ++++++++++++++++------
 1 file changed, 26 insertions(+), 10 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 1522eea..193c7d0 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
@@ -271,6 +271,8 @@ try_copy_propagate(const struct brw_device_info *devinfo,
     * vectors better.
     */
    src_reg value = *entry->value[0];
+   bool from_inst_has_source_modifiers = value.negate || value.abs;
+
    for (int i = 1; i < 4; i++) {
       /* This is equals() except we don't care about the swizzle. */
       if (value.file != entry->value[i]->file ||
@@ -311,6 +313,10 @@ try_copy_propagate(const struct brw_device_info *devinfo,
    if (inst->src[arg].negate)
       value.negate = !value.negate;
 
+   /* has_source_modifiers is using the modifiers at the current instruction,
+    * while from_inst_source_modifiers is using the modifiers of the source
+    * at the instruction the source comes from
+    */
    bool has_source_modifiers = value.negate || value.abs;
 
    /* gen6 math and gen7+ SENDs from GRFs ignore source modifiers on
@@ -322,7 +328,8 @@ try_copy_propagate(const struct brw_device_info *devinfo,
 
    if (has_source_modifiers &&
        value.type != inst->src[arg].type &&
-       !can_change_source_types(inst))
+       !can_change_source_types(inst) &&
+       from_inst_has_source_modifiers)
       return false;
 
    if (has_source_modifiers &&
@@ -340,7 +347,8 @@ try_copy_propagate(const struct brw_device_info *devinfo,
     * instead. See also resolve_ud_negate().
     */
    if (value.negate &&
-       value.type == BRW_REGISTER_TYPE_UD)
+       value.type == BRW_REGISTER_TYPE_UD &&
+       from_inst_has_source_modifiers)
       return false;
 
    /* Don't report progress if this is a noop. */
@@ -378,17 +386,25 @@ try_copy_propagate(const struct brw_device_info *devinfo,
 
    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.
+      /* We are propagating source modifiers from a safe instruction with a
+       * different type. If we got here, then we can just change the source
+       * and destination types of the current instruction or the instruction
+       * from we are propagating.
        */
-      assert(can_change_source_types(inst));
-      for (int i = 0; i < 3; i++) {
-         inst->src[i].type = value.type;
+      assert(can_change_source_types(inst) ||
+             !from_inst_has_source_modifiers);
+
+      if (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->dst.type = value.type;
-   } else
+   } else {
       value.type = inst->src[arg].type;
+   }
    inst->src[arg] = value;
    return true;
 }
-- 
2.1.4