[Mesa-dev] [PATCH 14/16] i965/fs: Allow flipping cond mod for negated arguments.

[Matt Turner]

On Thu, Feb 5, 2015 at 4:14 AM, Ian Romanick <idr at freedesktop.org> wrote:
> On 01/20/2015 01:31 AM, Matt Turner wrote:
>> @@ -73,10 +72,14 @@ opt_cmod_propagation_local(fs_visitor *v, bblock_t *block)
>>               scan_inst->dst.reg == inst->src[0].reg &&
>>               scan_inst->dst.reg_offset == inst->src[0].reg_offset &&
>>               !scan_inst->is_partial_write()) {
>> +            enum brw_conditional_mod cond =
>> +               inst->src[0].negate ? brw_invert_cmod(inst->conditional_mod)
>> +                                   : inst->conditional_mod;
>> +
>>              if (scan_inst->can_do_cmod() &&
>>                  ((!read_flag && scan_inst->conditional_mod == BRW_CONDITIONAL_NONE) ||
>> -                 scan_inst->conditional_mod == inst->conditional_mod)) {
>> -               scan_inst->conditional_mod = inst->conditional_mod;
>> +                 scan_inst->conditional_mod == cond)) {
>> +               scan_inst->conditional_mod = cond;
>
> I've been adding support for AND.nz here, and a question came up.  Does
> this work correctly if scan_inst is a CMP and the result register is
> used elsewhere?  Won't this turn
>
> cmp.l.f0(8)     g3<1>D          g3<8,8,1>F      g4<8,8,1>F
> mov.z.f0(8)     null            g3<8,8,1>D
> and(8)          g8<1>F          g3<8,8,1>UW     0x3f800000
>
> into
>
> cmp.ge.f0(8)    g3<1>D          g3<8,8,1>F      g4<8,8,1>F
> and(8)          g8<1>F          g3<8,8,1>UW     0x3f800000

We only allow moves with NZ to be handled, because handling them means
just removing them, so the example given isn't possible.

> For AND, we generate a lot of and.z for ir_unop_not in
> fs_visitor::emit_bool_to_cond_code.  I want to do the same cmod
> inversion for that, but it doesn't seem safe.  I think I have another
> way to deal with this for ir_unop_not, but either I don't really
> understand this code or there's already a bug here.

Yeah, handling AND.Z seems harder (seems like you would have to handle
the case you mention).

Lots of shaders do really annoying things like

cmp   dst src0 src1
and.z  null dst
(+f0) inst0
and.nz null dst
(+f0) inst1
and.z null dst
(+f0) inst2

It would be difficult to propagate things back into the CMP, and
that's not actually what we want. I was thinking about some kind of
opcode that just signifies that subsequent uses of the flag should be
reversed (predicate inverse). I haven't tried it so I'm sure I haven't
thought out all of the details.

In that case, we'd end up with something like

cmp dst src0 src1
invert flag
(+f0) inst0
keep flag
(+f0) inst1
invert flag
(+f0) inst2

and after some kind of propagation pass, ultimately:

cmp dst src0 src1
(-f0) inst0
(+f0) inst1
(-f0) inst2

It's somewhere on my todo list, but I'm not planning to work on it soon.