[Mesa-dev] [PATCH] i965/fs: Set pixel/sample mask for compute shaders atomic ops

[Jordan Justen]

On 2015-02-19 21:40:37, Ben Widawsky wrote:
> On Thu, Feb 19, 2015 at 03:42:05PM -0800, Jordan Justen wrote:
> > For fragment programs, we pull this mask from the payload header. The same
> > mask doesn't exist for compute shaders, so we set all bits to enabled.
> > 
> > Note: this mask is ANDed with the execution mask, so some channels may not end
> > up issuing the atomic operation.
> > 
> > Signed-off-by: Jordan Justen <jordan.l.justen at intel.com>
> > Cc: Ben Widawsky <ben at bwidawsk.net>
> > Cc: Francisco Jerez <currojerez at riseup.net>
> 
> Just add to the commit message that this is needed specifically because compute
> is invoked as SIMD16 (and perhaps reference the other commits?) and it's:
> Reviewed-by: Ben Widawsky <ben at bwidawsk.net>

Good idea. I'll add those.

> Sorry it advance...
> we may as well just go for 0xffffffff in case we ever support SIMD32.

I had been setting all 32-bits previously. I mentioned to you that I
thought this was needed for SIMD32. I wanted to double check it before
sending the patch out. I think I found the field for IVB in the PRM:

IVB Vol 4 Part 1 3.9.9.9 Message Header
Pixel/Sample Mask

...and it looks like it is only 16-bits. Maybe Francisco can confirm
that I got it right.

I couldn't find this same information in the HSW PRMs.

I'm not sure what that means for SIMD32.

-Jordan

> > ---
> >  While it's fresh in our minds. :)
> > 
> >  This seems to work for gen7 & gen8 CS. For CS simd16, we need the
> >  0xffff change, but it seems to work fine for simd8 as well.
> > 
> >  I also tested gen8 (simd8vs), and there were no piglit regressions.
> > 
> >  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> > index 24cc118..960a0aa 100644
> > --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> > @@ -2998,9 +2998,9 @@ fs_visitor::emit_untyped_atomic(unsigned atomic_op, unsigned surf_index,
> >         * mask sent in the header to compute the actual set of channels that execute
> >         * the atomic operation.
> >         */
> > -      assert(stage == MESA_SHADER_VERTEX);
> > +      assert(stage == MESA_SHADER_VERTEX || stage == MESA_SHADER_COMPUTE);
> >        emit(MOV(component(sources[0], 7),
> > -               brw_imm_ud(0xff)))->force_writemask_all = true;
> > +               brw_imm_ud(0xffff)))->force_writemask_all = true;
> >     }
> >     length++;
> >  
> > @@ -3061,9 +3061,9 @@ fs_visitor::emit_untyped_surface_read(unsigned surf_index, fs_reg dst,
> >         * mask sent in the header to compute the actual set of channels that execute
> >         * the atomic operation.
> >         */
> > -      assert(stage == MESA_SHADER_VERTEX);
> > +      assert(stage == MESA_SHADER_VERTEX || stage == MESA_SHADER_COMPUTE);
> >        emit(MOV(component(sources[0], 7),
> > -               brw_imm_ud(0xff)))->force_writemask_all = true;
> > +               brw_imm_ud(0xffff)))->force_writemask_all = true;
> >     }
> >  
> >     /* Set the surface read offset. */
> > -- 
> > 2.1.4
> >