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

[Jason Ekstrand]

On Thu, Sep 24, 2015 at 11:54 AM, Alejandro Piñeiro
<apinheiro at igalia.com> wrote:
> 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
>
> v3:
> - Move the check back to the original place, to keep things
>   tidy, as suggested by Jason Ekstrand
>
> v4:
> - Fixed missing is_single_value_swizzle() as pointed by Jason Ekstrand

Good work!

Reviewed-by: Jason Ekstrand <jason.ekstrand at intel.com>

> ---
>
>> > +       value.file == UNIFORM &&
>> > +       !composed_swizzle)
>
>> Where did the is_single_value_swizzle() go?
>
> I cleaned too much. Sorry for the noise.
>
>
>  src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 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..5b6444e 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
> @@ -325,7 +325,11 @@ 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)
> +   unsigned composed_swizzle = brw_compose_swizzle(inst->src[arg].swizzle,
> +                                                   value.swizzle);
> +   if (inst->is_3src() &&
> +       value.file == UNIFORM &&
> +       !brw_is_single_value_swizzle(composed_swizzle))
>        return false;
>
>     if (inst->is_send_from_grf())
> @@ -380,8 +384,7 @@ 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);
> +   value.swizzle = composed_swizzle;
>     if (has_source_modifiers &&
>         value.type != inst->src[arg].type) {
>        assert(can_change_source_types(inst));
> --
> 2.1.4
>