[Mesa-dev] [PATCH 04/23] i965/fs: Don't emit dumb SEL

[Ian Romanick]

On 03/24/2015 05:01 AM, Tapani Pälli wrote:
> 
> On 03/20/2015 10:58 PM, Ian Romanick wrote:
>> From: Ian Romanick <ian.d.romanick at intel.com>
>>
>> With previous changes to emit more ir_triop_csel, the i965 channel
>> expressions pass can generate sequences like:
>>
>>      mov(8)          g45<1>F      1F
>>      (+f0) sel(8)    g40<1>F      g45<8,8,1>F 1F
>>
>> There are no shader-db changes now, but this prevents a large number of
>> regressions from a later commit (glsl: Optimize certain if-statements to
>> ir_triop_csel).
>>
>> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
>> Cc: Tapani Pälli <tapani.palli at intel.com>
>> ---
>>   src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
>> b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
>> index e39dab3..1fbef5f 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
>> @@ -708,6 +708,15 @@ fs_visitor::visit(ir_expression *ir)
>>         break;
>>
>>      case ir_triop_csel:
>> +      /* After splitting an expression like 'v = csel(cond, vec4(a,
>> b, c, 1),
>> +       * vec4(d, e, f, 1))', there will be a 'v.w = csel(cond, 1,
>> 1)'.  Detect
>> +       * this, and avoid emitting the spurious SEL.
>> +       */
>> +      if (ir->operands[1]->equals(ir->operands[2])) {
>> +         ir->operands[1]->accept(this);
>> +         return;
> 
> The result is correct but why is this done in the backend? Couldn't the
> later commit mentioned for glsl already detect this or maybe additional
> optimization pass there?

I had assumed there was already an algebraic optimization for this case
that, for whatever reason, just wasn't triggering here.  I went to go
find it, and it does not exist... it looks like replacing this code with
a similar change to src/glsl/opt_algebraic.cpp is equally effective.
I'll do that instead.

>> +      }
>> +
>>         if (try_opt_frontfacing_ternary(ir))
>>            return;
>>
>>
> 
> // Tapani