[Mesa-dev] gl_nir_lower_samplers_as_deref vs drawpixels lowering

[Kenneth Graunke]

Yeah...I thought the drawpixels lowering set up samplers directly with
explicit bindings.  There should be no need to call it twice.

We can't handle setting textures_used in nir_shader_gather_info because
that is called both before and after lowering, and some of the
information needed is no longer available on subsequent iterations.
I moved it to gl_nir_lower_samplers_as_deref so it would happen once
when all of the information was still present; lowering passes would
patch it up as needed.

Maybe the second call got introduced when making the finalize_nir()
hook, and it isn't actually needed?  (I haven't done the archaeology
yet...)

On Monday, November 25, 2019 2:24:55 AM PST Connor Abbott wrote:
> Why are you calling gl_nir_lower_samplers_as_deref twice? The entire
> point of it is to deal with the crazy legacy GL model where samplers
> are "just normal uniforms" that can be embedded in structs and calling
> glUniform() can update the sampler binding. It doesn't touch samplers
> with an explicit binding at all. It does set nir->info.textures_used,
> but that's a small part of what it does. So while you certainly could
> make it re-entrant, it really wouldn't make any sense to call it twice
> as it isn't necessary for any mesa-internal samplers besides the
> (trivial) updating textures_used. So I'd say that the best course of
> action is to just update nir->info.textures_used by yourself in the
> drawpix lowering pass and not call gl_nir_lower_samplers_as_deref.
> Maybe longer-term the solution is to fill that out in nir_gather_info?
> 
> Connor
> 
> On Mon, Nov 25, 2019 at 6:56 AM Dave Airlie <airlied at gmail.com> wrote:
> >
> > I was asked to use some newer radeonsi code in my tgsi info gathering
> > wrapper for NIR. One of the things it does is use
> > nir->info.textures_used.
> >
> > Now with the piglit test draw-pixel-with-texture there is a reentrancy
> > issue with the passes.
> >
> > We create a shader and gl_nir_lower_samplers_as_deref get called on
> > it, this sets nir->info.textures_used to 1, and it also lowers all
> > texture derefs in the shader.
> >
> > The shader gets used in a variant later for drawpixels, the drawpixels
> > lowering then adds it's own "drawpix" sampler an accesses it with
> > derefs. Then gl_nir_lower_samplers_as_deref gets called again for the
> > whole shader in finalisation, but this time the first set of derefs
> > have already been lowered so it only lowers the new drawpix ones, and
> > sets nir->info.textures_used to 2 (it's a bitfield), it should be 3.
> >
> > Are the other drivers seeing this? any ideas on what needs to be
> > fixed, the nir sampler lowering could I suppose record
> > nir->info.textures_used for non-deref textures as well.
> >
> > Dave.
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20191125/bdd7de9c/attachment.sig>