On Tue, Dec 22, 2015 at 9:55 PM, Rob Clark <robdcl...@gmail.com> wrote: > On Tue, Dec 22, 2015 at 9:47 PM, Connor Abbott <cwabbo...@gmail.com> wrote: >> On Tue, Dec 22, 2015 at 9:02 PM, Rob Clark <robdcl...@gmail.com> wrote: >>> On Mon, Dec 21, 2015 at 1:48 PM, Jason Ekstrand <ja...@jlekstrand.net> >>> wrote: >>>> >>>> I think two different concepts of ownership are getting conflated here: >>>> Right/responsibility to delete and right to modify. >>>> >>>> The way I understand it, gallium, as it stands, gives neither to the >>>> driver. >>>> A back-end using NIR requires the right to modify but who deletes it >>>> doesn't >>>> ultimately matter. I think it's dangerous to pass one of these rights to >>>> the driver and not the other but we need to think about both. >>> >>> yeah, uneasy about driver modifying the IR if the state tracker is >>> still going to potentially spin off variants of the IR.. that sounds >>> like madness. >>> >>> The refcnt'ing I proposed does deal w/ right to modify vs delete via >>> nir_shader(_is)_mutable() which returns something that is guaranteed >>> to be safe to modify (ie. has only a single reference) >>> >>>> What I'm trying to say is that we have two options here: >>>> >>>> 1) gallium passes IR to the back-end that it is free to modify and is >>>> required to delete when it's done. >>> >>> with refcnt'ing, s/delete/unref/ >>> >>> The idea is, the st transfers ownership of the reference it passes to >>> the driver. If the st wants to hang on to a reference itself, it must >>> increment the refcnt before passing to the driver (backend). >>> >>> Without refcnt'ing, I suppose we could (since we don't have to follow >>> TGSI semantics), just decree that the driver always takes ownership of >>> the copy passed in, and if the st wants to hang on to a copy too, then >>> it must clone. I suppose this would work well enough for >>> freedreno/vc4, which both end up generating variants later. It does >>> force an extra clone for drivers that immediately translate into their >>> own backend IR and don't need to keep the NIR around, for example. >>> Maybe that is not worth caring about (since at this point it is >>> hypothetical). >> >> While always cloning does have this disadvantage, I don't think it's >> really relevant here. Even if the driver throws away the NIR >> immediately after consuming it, almost invariably it's going to want >> to modify it. The generic NIR passed in by the state tracker (other >> IR -> NIR + some optimizations) is almost never going to be the same >> as the NIR after going through driver-specific lowering passes, which >> means that drivers are never going to want a read-only version of the >> IR. In light of that, I think making the driver own the IR passed in >> seems like the most sensible thing. > > well, unless the driver is already doing it's own lowering in it's own > native IR..
Well, if you're not doing any lowering in NIR, then you aren't really taking any advantage of it. I can't see a plausible scenario where all the lowering is done in the driver's own IR -- and as soon as you do anything in NIR, you need the driver-owns-IR semantics. > > Maybe it is too much of a hypothetical.. I still think refcnt'ing > gives some nice flexibility to deal with various scenarios, and having > to call nir_shader_unref() isn't so much of a burden. Still, I can't see how this flexibility is at all useful, and it seems like overkill since the driver will always want a mutable version of the IR anyways. > > BR, > -R > >>> >>> (I guess nouveau is the one driver, that if it ever consumed NIR, >>> would translate immediately into it's own backend IR?) >>> >>>> 2) gallium passes read-only IR to the back-end and it always makes a copy. >>> >>> This is how it is w/ TGSI, but I think with NIR we are free to make a >>> clean break. And we *definitely* want to avoid >>> the-driver-always-copies semantics.. >>> >>>> It sounds like, from what Marek is saying, that gallium is currently doing >>>> (2) and changing it to (1) would be painful. I think reference counting is >>>> more like an awkward option 1.5 than option 3. Reference counting would >>>> mean that gallium passes a reference to the driver which it is expected to >>>> unref but may keep a second reference if it wants to keep the driver from >>>> modifying it. Then the driver may or may not make a copy based on the >>>> number of references. Why don't we just make it explicit and add a >>>> read-only bit and call it a day. >>>> >>>> One of the reasons I don't like passing a reference is that it effectively >>>> puts allocation and freeing in different components of the driver. >>> >>> With refcnt'ing you should talk in terms of ref/unref rather than >>> allocate/free.. imho. Although maybe that is what you meant. (In >>> which case, yes, that was my idea, that passing in to driver transfers >>> ownership of the passed reference.) >>> >>>> This >>>> means that if and driver doesn't care at all about the shader that gets >>>> passed in, it still has to under it to avoid a memory leak. You can't have >>>> the driver take the reference because then, either it comes in with a >>>> recount of 0 and should have been deleted, or the "can I modify this" check >>>> becomes "recount <= 2" which makes no sense. >>> >>> hmm, no, if ownership of the reference is transferred to the driver, >>> then it becomes "refcount == 1" (and refcount == 0 should be an >>> assert, because something has gone horribly wrong) >>> >>> BR, >>> -R _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
