On Thu, Nov 5, 2015 at 5:58 PM, Jason Ekstrand <ja...@jlekstrand.net> wrote: > On Thu, Nov 5, 2015 at 10:53 AM, Rob Clark <robdcl...@gmail.com> wrote: >> On Thu, Nov 5, 2015 at 1:13 PM, Jason Ekstrand <ja...@jlekstrand.net> wrote: >>> On Sat, Oct 24, 2015 at 10:07 AM, Rob Clark <robdcl...@gmail.com> wrote: >>>> From: Rob Clark <robcl...@freedesktop.org> >>>> >>>> For gallium, at least, we'll need this to manage shader's lifetimes, >>>> since in some cases both the driver and the state tracker will need >>>> to hold on to a reference for variant managing. >>>> >>>> Use nir_shader_mutable() before doing any IR opt/lowering/etc, to >>>> ensure you are not modifying a copy someone else is also holding a >>>> reference to. In this way, unnecessary nir_shader_clone()s are >>>> avoided whenever possible. >>>> >>>> TODO: Any places missed to s/ralloc_free()/nir_shader_unref()/ outside >>>> of freedreno/ir3? >>> >>> I talked with Ken about this a few days ago, and it doesn't seem like >>> a good idea. Adding reference counting to NIR means mixing two very >>> different memory management models. NIR already uses the ralloc model >>> where a shader is tied to a context. Allowing both ralloc and >>> refcount models is a bad idea because freeing a context could cause >>> the shader to go out-of-scope early from the refcount perspective and >>> refcounting means you don't know what thread the shader is deleted in >>> which is really bad for ralloc. The only way we could do this is if >>> we only used ralloc internally to NIR and disallowed parenting a >>> nir_shader to anything. I don't really think we want this, at least >>> not for the i965 driver. >>> >>> The best option here would probably be to make a little flyweight >>> wrapper object for TGSI to use that acts as the ralloc context for a >>> NIR shader and is, itself, reference-counted. You still have to >>> provide similar guarantees (said object can never have a parent), but >>> it would keep those guarantees obvious. >> >> Hmm, maybe a better idea still (and simpler) would be to just disallow >> nir_shader_ref() if parent memctx is not null? > > Yeah, we could do that. However, we have no way of doing a similar > check to say you can't reparent it if refcount > 1.
well, we could always add a nir_shader_reparent().. current state, this all seems a bit hypothetical since nir_shader_create() is always called w/ a NULL parent memctx.. >> This way we can keep the centralized checks for refcnt>1 in the >> nir-pass runner function/macro/whatever.. > > Meh. Call the "get me a unique shader" function in your backend > before you do any transformations on it.. There's no reason to call > it multiple times. If the refcount goes from 1 to > 1 during your > optimization loop, you're sunk anyway. I do actually do this now, but there are places where mesa st will do lowering passes, etc. I do really like the kill-all-birds-with-one-stone approach of putting the check in the nir-pass runner. BR, -R _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
