[Mesa-dev] [PATCH v2] i965/vec4: check swizzle before discarding a uniform on a 3src operand

[Alejandro Piñeiro]

Without this commit, copy propagation is discarded if it involves
a uniform with an instruction that has 3 sources. But 3 sourced
instructions can access scalar values.

For example, this is what vec4_visitor::fix_3src_operand() is already
doing:

   if (src.file == UNIFORM && brw_is_single_value_swizzle(src.swizzle))
      return src;

Shader-db results (unfiltered) on NIR:
total instructions in shared programs: 6259650 -> 6241985 (-0.28%)
instructions in affected programs:     812755 -> 795090 (-2.17%)
helped:                                7930
HURT:                                  0

Shader-db results (unfiltered) on IR:
total instructions in shared programs: 6445822 -> 6441788 (-0.06%)
instructions in affected programs:     296630 -> 292596 (-1.36%)
helped:                                2533
HURT:                                  0

v2:
- Updated commit message, using Matt Turner suggestions
- Move the check after we've created the final value, as Jason
  Ekstrand suggested
- Clean up the condition
---

Removed RFC as both Matt Turner and Jason Ekstrand accepted the change.

I updated the patch as the condition was just ugly. As I made that change
I also included Jason suggestion to move the check after the swizzle
on the final value is computed. I know that the first version got a
"Reviewed" by Jason, but due all these change, I prefer to be conservative,
and ask for a new review.

Thanks in advance.

 src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 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 d3f0ddd..ce4d39d 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
@@ -325,9 +325,6 @@ try_copy_propagate(const struct brw_device_info *devinfo,
        inst->opcode == SHADER_OPCODE_GEN4_SCRATCH_WRITE)
       return false;
 
-   if (inst->is_3src() && value.file == UNIFORM)
-      return false;
-
    if (inst->is_send_from_grf())
       return false;
 
@@ -373,6 +370,13 @@ try_copy_propagate(const struct brw_device_info *devinfo,
    }
 
    /* Build the final value */
+   value.swizzle = brw_compose_swizzle(inst->src[arg].swizzle,
+                                       value.swizzle);
+   /* One last check based on the final swizzle */
+   if (inst->is_3src() && value.file == UNIFORM &&
+       !brw_is_single_value_swizzle(value.swizzle))
+      return false;
+
    if (inst->src[arg].abs) {
       value.negate = false;
       value.abs = true;
@@ -380,8 +384,6 @@ try_copy_propagate(const struct brw_device_info *devinfo,
    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) {
       assert(can_change_source_types(inst));
-- 
2.1.4