[Mesa-dev] gl_nir_lower_samplers_as_deref vs drawpixels lowering

[Marek Olšák]

The way shader variants work in st/mesa is that NIR is generated,
finalized, and stored in the cache. This helps the most common case when
there is only one variant. If shader variants make changes to NIR, like
adding samplers, uniforms, and inputs, it needs to be finalized again,
which means many passes have to be run again.

Marek

On Mon, Nov 25, 2019 at 1:29 PM Kenneth Graunke <kenneth at whitecape.org>
wrote:

> 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 --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20191125/d96647be/attachment.html>