[Mesa-dev] [RFC v1] i965/vec4: add a swizzle check on the uniform bail condition

[Jason Ekstrand]

On Wed, Sep 23, 2015 at 1:15 PM, Matt Turner <mattst88 at gmail.com> wrote:
> On Wed, Sep 23, 2015 at 1:07 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
>> On Sep 23, 2015 12:29, "Alejandro Piñeiro" <apinheiro at igalia.com> wrote:
>>> 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..400a13a 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,8 @@ 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)
>>> +   if (inst->is_3src() && value.file == UNIFORM &&
>>> +
>>> !brw_is_single_value_swizzle(brw_compose_swizzle(inst->src[arg].swizzle,
>>> value.swizzle)))
>>
>> This is a case where we probably want to do the check after we've created
>> the "final" value.  The question that's being asked is "is this final value
>> allowed?"
>
> I think that's what the brw_compose_swizzle() is doing -- combining
> the swizzles and then asking if the result is single-valued.

Yes, that is what it's doing.  I guess I don't care too much where in
the pass we do this check it just makes sense to me to do it later.
Either way,

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

>> Also, we emitted it before with the is_visitor code. What checks are we
>> doing there to make sure we only emitted the uniform directly if it was OK
>> to do so?
>
> Look at vec4_visitor::fix_3src_operand():
>
>    if (src.file == UNIFORM && brw_is_single_value_swizzle(src.swizzle))
>       return src;
>
> so it was doing the right thing.

Right.  Makes sense.