[Mesa-dev] [PATCH] i965: Fix SHADER_OPCODE_UNTYPED_SURFACE_WRITE to mark written surfaces

[Iago Toral]

On Tue, 2015-05-05 at 16:41 +0300, Francisco Jerez wrote:
> Iago Toral Quiroga <itoral at igalia.com> writes:
> 
> > We can do this when the surface index is an immediate, as we do for
> > reads. Otherwise the visitor should handle this.
> > ---
> > Francisco, maybe you choose not to do this for a reason? It seems a bit
> > inconsistent with the reads though. Also, other read/write messages seem
> > to mark surfaces in the generator when the surface index is an immediate
> > and leave the visitor to handle non-immediate scenarios.
> >
> 
> Yeah, this deviation from the other untyped surface opcodes was
> intentional, actually I sent a patch to remove the implicit call to
> brw_mark_surface_used() for untyped surface reads and atomics which I
> haven't landed yet [1].

Good to know, since I'll be using these functions I'll make sure I
always mark used surfaces in the visitor code then.

>   The reason is that the surface index may not be
> an immediate, so the visitor cannot in general rely on the generator to
> do it.  Whether the generator can do it or not is even dependent on the
> extent that optimization passes are able to simplify the program, so it
> could lead to hard to debug errors.  It seems more consistent and less
> error-prone to have the visitor handle this unconditionally rather than
> sharing the responsibility between visitor and generator depending on
> the situation.

Yes, makes sense.

Thanks,
Iago

> [1] http://lists.freedesktop.org/archives/mesa-dev/2015-May/083278.html
> 
> >  src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> > index b8b8ed1..cf4b9ae 100644
> > --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> > @@ -2247,6 +2247,8 @@ fs_generator::generate_code(const cfg_t *cfg, int dispatch_width)
> >           assert(src[2].file == BRW_IMMEDIATE_VALUE);
> >           brw_untyped_surface_write(p, src[0], src[1],
> >                                     inst->mlen, src[2].dw1.ud);
> > +         if (src[1].file == BRW_IMMEDIATE_VALUE)
> > +            brw_mark_surface_used(prog_data, src[1].dw1.ud);
> >           break;
> >  
> >        case SHADER_OPCODE_TYPED_ATOMIC:
> > -- 
> > 1.9.1