i965/vec4: refactor brw_vec4_copy_propagation.
authorAlejandro Piñeiro <apinheiro@igalia.com>
Wed, 16 Sep 2015 15:19:50 +0000 (17:19 +0200)
committerAlejandro Piñeiro <apinheiro@igalia.com>
Tue, 22 Sep 2015 17:30:18 +0000 (19:30 +0200)
Now it is more similar to brw_fs_copy_propagation, with three
clear stages:

1) Build up the value we are propagating as if it were the source of a
single MOV:
2) Check that we can propagate that value
3) Build the final value

Previously everything was somewhat messed up, making the
implementation on some specific cases, like knowing if you can
propagate from a previous instruction even with type mismatches, even
messier (for example, with the need of maintaining more of one
has_source_modifiers). The refactoring clears stuff, and gives
support to this mentioned use case without doing anything extra
(for example, only one has_source_modifiers is used).

Shader-db results for vec4 programs on Haswell:
total instructions in shared programs: 1683842 -> 1669037 (-0.88%)
instructions in affected programs:     739837 -> 725032 (-2.00%)
helped:                                6237
HURT:                                  0

v2: using 'arg' index to get the from inst was wrong
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
v5: change the approach for a refactoring
v6: tweaked comments

Reviewed-by: Jason Ekstrand <jason.ekstrand@intel.com>
src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp

index 1522eeabb1cb7a2b876e70f46b64a50c6cb5405e..d3f0ddde258f861fec240588df3b706c00bcce6b 100644 (file)
@@ -265,6 +265,9 @@ try_copy_propagate(const struct brw_device_info *devinfo,
                    vec4_instruction *inst,
                    int arg, struct copy_entry *entry)
 {
+   /* Build up the value we are propagating as if it were the source of a
+    * single MOV
+    */
    /* For constant propagation, we only handle the same constant
     * across all 4 channels.  Some day, we should handle the 8-bit
     * float vector format, which would let us constant propagate
@@ -291,9 +294,9 @@ try_copy_propagate(const struct brw_device_info *devinfo,
    for (int i = 0; i < 4; i++) {
       s[i] = BRW_GET_SWZ(entry->value[i]->swizzle, i);
    }
-   value.swizzle = brw_compose_swizzle(inst->src[arg].swizzle,
-                                       BRW_SWIZZLE4(s[0], s[1], s[2], s[3]));
+   value.swizzle = BRW_SWIZZLE4(s[0], s[1], s[2], s[3]);
 
+   /* Check that we can propagate that value */
    if (value.file != UNIFORM &&
        value.file != GRF &&
        value.file != ATTR)
@@ -304,13 +307,6 @@ try_copy_propagate(const struct brw_device_info *devinfo,
       return false;
    }
 
-   if (inst->src[arg].abs) {
-      value.negate = false;
-      value.abs = true;
-   }
-   if (inst->src[arg].negate)
-      value.negate = !value.negate;
-
    bool has_source_modifiers = value.negate || value.abs;
 
    /* gen6 math and gen7+ SENDs from GRFs ignore source modifiers on
@@ -376,19 +372,27 @@ try_copy_propagate(const struct brw_device_info *devinfo,
       }
    }
 
+   /* Build the final value */
+   if (inst->src[arg].abs) {
+      value.negate = false;
+      value.abs = true;
+   }
+   if (inst->src[arg].negate)
+      value.negate = !value.negate;
+
+   value.swizzle = brw_compose_swizzle(inst->src[arg].swizzle,
+                                       value.swizzle);
    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
+   } else {
       value.type = inst->src[arg].type;
+   }
+
    inst->src[arg] = value;
    return true;
 }