[Mesa-dev] [RFC] i965: alternative to memctx for cleaning up nir variants

[Rob Clark]

On Mon, Dec 21, 2015 at 1:48 PM, Jason Ekstrand <jason at 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).

(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