[Mesa-dev] [PATCH 2/5] i965/vec4: adding vec4_cmod_propagation optimization

[Matt Turner]

On Tue, Oct 13, 2015 at 1:49 AM, Alejandro Piñeiro <apinheiro at igalia.com> wrote:
> On 13/10/15 03:10, Matt Turner wrote:
>> Looks like this is causing an intermittent failure on HSW in our
>> Jenkins system (but I'm not able to reproduce locally) and a
>> consistent failure on G45. I'll investigate that.
>
> Ok, will hold on, and meanwhile I will try to reproduce the problem on
> HSW. Unfortunately I don't have any G45 available, so I will not be able
> to help on that. Thanks for taking a look there.

Okay, I think I see what's going on:

--- good        2015-10-13 13:34:40.753357253 -0700
+++ bad 2015-10-13 13:36:06.114290094 -0700
-and(8)          g6<1>.xD        g6<4,4,1>.xD    1D              { align16 };
-cmp.nz.f0(8)    null            -g6<4,4,1>.xD   0D              { align16 };
+and.nz.f0(8)    g6<1>.xD        g6<4,4,1>.xD    1D              { align16 };

We're propagating a .nz conditional mod from a CMP with a null dest
(that has a writemask of .xyzw) to an AND that has a writemask of only
.x, so only the .x channel of the flag is now being updated.

I think for now the thing to do is add

   (inst->dst.writemask & ~scan_inst->dst.writemask) != 0)

to the list of conditions under which we bail out. That is, if the
instruction we want to propagate the cmod onto writes fewer channels,
we can't do the optimization.

With that, the block should look like:

     if ((scan_inst->predicate && scan_inst->opcode != BRW_OPCODE_SEL) ||
         scan_inst->dst.reg_offset != inst->src[0].reg_offset ||
         (scan_inst->dst.writemask != WRITEMASK_X &&
          scan_inst->dst.writemask != WRITEMASK_XYZW) ||
         (scan_inst->dst.writemask == WRITEMASK_XYZW &&
          inst->src[0].swizzle != BRW_SWIZZLE_XYZW) ||
         (inst->dst.writemask & ~scan_inst->dst.writemask) != 0)
        break;

The good news is that, unless I've done something wrong, this doesn't
affect any shaders in shader-db on ILK or HSW (I only tested those
two, but I expect the results are the same everywhere). Apparently
this is a pretty rare case.

With that change, my R-b still stands, though we should have a unit
test for this case as well in 3/5.

Thanks!
Matt