Re: [Mesa-dev] gl_nir_lower_samplers_as_deref vs drawpixels lowering
On Tue, 26 Nov 2019 at 05:19, Marek Olšák wrote: > > 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. > So this is what's happening, we are calling the gl_nir lowering pass twice, once for the main shader, once for the variant, the call for the variant messes up. Dave. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] gl_nir_lower_samplers_as_deref vs drawpixels lowering
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 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 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. > > > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] gl_nir_lower_samplers_as_deref vs drawpixels lowering
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 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. > signature.asc Description: This is a digitally signed message part. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] gl_nir_lower_samplers_as_deref vs drawpixels lowering
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 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. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
