[Mesa-dev] [PATCH v2 01/16] i965: Don't copy propagate sat MOVs into LOAD_PAYLOAD

[Kristian Høgsberg]

On Thu, Nov 13, 2014 at 8:49 PM, Matt Turner <mattst88 at gmail.com> wrote:
> On Thu, Nov 13, 2014 at 4:28 PM, Kristian Høgsberg <krh at bitplanet.net> wrote:
>> The LOAD_PAYLOAD opcode can't saturate its sources, so skip
>> saturating MOVs.  The register coalescing after lower_load_payload()
>> will clean up the extra MOVs.
>>
>> Signed-off-by: Kristian Høgsberg <krh at bitplanet.net>
>> ---
>>  src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
>> index e1989cb..87ea9c2 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
>> @@ -454,8 +454,12 @@ fs_visitor::try_constant_propagate(fs_inst *inst, acp_entry *entry)
>>        val.effective_width = inst->src[i].effective_width;
>>
>>        switch (inst->opcode) {
>> -      case BRW_OPCODE_MOV:
>>        case SHADER_OPCODE_LOAD_PAYLOAD:
>> +         /* LOAD_PAYLOAD can't sat its sources. */
>> +         if (entry->saturate)
>> +            break;
>> +         /* Otherwise, fall through */
>> +      case BRW_OPCODE_MOV:
>>           inst->src[i] = val;
>>           progress = true;
>>           break;
>> --
>> 2.1.0
>
> This looks like the same patch as 01/14 in the previous series. I
> suggested a better approach there:
>
> At the beginning of fs_visitor::try_constant_propagate, if
> (entry->saturate) return false, or just saturate the argument and
> proceed. We don't want to be propagating the result of a mov.sat 4.0
> into anything without saturating the result first.

Fixed in v3.

Kristian