[Mesa-dev] [RFC] NIR serialization

[Connor Abbott]

On Thu, Sep 14, 2017 at 10:15 PM, Connor Abbott <cwabbott0 at gmail.com> wrote:
> me too :) I'll push my stuff now
>
> On Thu, Sep 14, 2017 at 8:58 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
>> On Thu, Sep 14, 2017 at 12:40 PM, Connor Abbott <cwabbott0 at gmail.com> wrote:
>>>
>>> On Tue, Sep 12, 2017 at 2:09 PM, Jason Ekstrand <jason at jlekstrand.net>
>>> wrote:
>>> > On Tue, Sep 12, 2017 at 10:12 AM, Ian Romanick <idr at freedesktop.org>
>>> > wrote:
>>> >>
>>> >> On 09/11/2017 11:17 PM, Kenneth Graunke wrote:
>>> >> > On Monday, September 11, 2017 9:23:05 PM PDT Ian Romanick wrote:
>>> >> >> On 09/08/2017 01:59 AM, Kenneth Graunke wrote:
>>> >> >>> On Thursday, September 7, 2017 4:26:04 PM PDT Jordan Justen wrote:
>>> >> >>>> On 2017-09-06 14:12:41, Daniel Schürmann wrote:
>>> >> >>>>> Hello together!
>>> >> >>>>> Recently, we had a small discussion (off the list) about the NIR
>>> >> >>>>> serialization, which was previously discussed in [RFC]
>>> >> >>>>> ARB_gl_spirv
>>> >> >>>>> and
>>> >> >>>>> NIR backend for radeonsi.
>>> >> >>>>>
>>> >> >>>>> As this topic could be interesting to more people, I would like
>>> >> >>>>> to
>>> >> >>>>> share, what was talked about so far (You might want to read from
>>> >> >>>>> bottom up).
>>> >> >>>>>
>>> >> >>>>> TL;DR:
>>> >> >>>>> - NIR serialization is in demand for shader cache
>>> >> >>>>> - could be done either directly (NIR binary form) or via SPIR-V
>>> >> >>>>> - Ian et al. are working on GLSL IR -> SPIR-V transformation,
>>> >> >>>>> which
>>> >> >>>>> could be adapted for a NIR -> SPIR-V pass
>>> >> >>>>> - in NIR representation, some type information is lost
>>> >> >>>>> - thus, a serialization via SPIR-V could NOT be a glslang
>>> >> >>>>> alternative
>>> >> >>>>> (otoh, the GLSL IR->SPIR-V pass could), but only for spirv-opt
>>> >> >>>>> (if
>>> >> >>>>> the
>>> >> >>>>> output is valid SPIR-V)
>>> >> >>>>
>>> >> >>>> Ian,
>>> >> >>>>
>>> >> >>>> Tim was suggesting that we might look at serializing nir for the
>>> >> >>>> i965
>>> >> >>>> shader cache. Based on this email, it sounds like serialized nir
>>> >> >>>> would
>>> >> >>>> not be enough for the shader cache as some GLSL type info would be
>>> >> >>>> lost. It sounds like GLSL IR => SPIR-V would be good enough. Is
>>> >> >>>> that
>>> >> >>>> right?
>>> >> >>>>
>>> >> >>>> I don't think we have a strict requirement for the GLSL IR =>
>>> >> >>>> SPIR-V
>>> >> >>>> path for GL 4.6, right? So, this is more of a 'nice-to-have'?
>>> >> >>>>
>>> >> >>>> I'm not sure we'd want to make i965 shader cache depend on a
>>> >> >>>> nice-to-have feature. (Unless we're pretty sure it'll be available
>>> >> >>>> soon.)
>>> >> >>>>
>>> >> >>>> But, it would be nice to not have to fallback to compiling the
>>> >> >>>> GLSL
>>> >> >>>> for i965 shader cache, so it would be worth waiting a little bit
>>> >> >>>> to
>>> >> >>>> be
>>> >> >>>> able to rely on a SPIR-V serialization of the GLSL IR.
>>> >> >>>>
>>> >> >>>> What do you suggest?
>>> >> >>>>
>>> >> >>>> -Jordan
>>> >> >>>
>>> >> >>> We shouldn't use SPIR-V for the shader cache.
>>> >> >>>
>>> >> >>> The compilation process for GLSL is: GLSL -> GLSL IR -> NIR -> i965
>>> >> >>> IRs.
>>> >> >>> Storing the content at one of those points, and later loading it
>>> >> >>> and
>>> >> >>> resuming the normal compilation process from that point...that's
>>> >> >>> totally
>>> >> >>> reasonable.
>>> >> >>>
>>> >> >>> Having a fallback for "some things in the cache but not all the
>>> >> >>> variants
>>> >> >>> we needed" suddenly take a different compilation pipeline, i.e.
>>> >> >>> SPIR-V
>>> >> >>> -> NIR -> ... seems risky.  It's a different compilation path that
>>> >> >>> we
>>> >> >>> don't normally use.  And one you'd only hit in limited
>>> >> >>> circumstances.
>>> >> >>> There's a lot of potential for really obscure bugs.
>>> >> >>
>>> >> >> Since we're going to expose exactly that path for GL_ARB_spirv /
>>> >> >> OpenGL
>>> >> >> 4.6, we'd better make sure it works always.  Right?
>>> >> >
>>> >> > In addition to the old pipeline:
>>> >> >
>>> >> > - GLSL from the app -> GLSL IR -> NIR -> i965 IR
>>> >> >
>>> >> > GL_ARB_spirv and OpenGL 4.6 add a second pipeline:
>>> >> >
>>> >> > - SPIR-V from the app -> NIR -> i965 IR
>>> >> >
>>> >> > Both of those absolutely have to work.  But these:
>>> >> >
>>> >> > - GLSL -> GLSL IR -> NIR -> SPIR-V -> NIR -> i965 IRs
>>> >> > - GLSL -> GLSL IR -> SPIR-V -> NIR -> i965 IRs
>>> >> >
>>> >> > aren't required to work, or even be supported.  It makes a lot of
>>> >> > sense
>>> >> > to support them - both for testing purposes, and as an alternative to
>>> >> > glslang, for a broader tooling ecosystem.
>>> >> >
>>> >> > The thing that concerns me is that if you use SPIR-V for the cache,
>>> >> > you
>>> >> > need these paths to not just work, but be _indistinguishable_ from
>>> >> > one
>>> >> > another:
>>> >> >
>>> >> > - GLSL -> GLSL IR -> NIR -> ...
>>> >> > - GLSL -> GLSL IR -> NIR -> SPIR-V, then SPIR-V -> NIR -> ...
>>> >> >
>>> >> > Otherwise the original compile and partially-cached recompile might
>>> >> > have
>>> >> > different properties.  For example, if the the SPIR-V step messes
>>> >> > with
>>> >> > variables or instruction ordering a little, it could trip up the loop
>>> >> > unroller so the original compiler gets unrolled, and the recompile
>>> >> > from
>>> >> > partial cache doesn't get unrolled.  I don't want to have to debug
>>> >> > that.
>>> >>
>>> >> That is a very compelling argument.  If we want Mesa to be an
>>> >> alternative to glslang, I think we would like to have that property,
>>> >> but
>>> >> it's not a hard requirement for that use case.
>>> >
>>> >
>>> > I also find that argument rather compelling.  The SPIR-V -> NIR pass is
>>> > *not* a simple pass.  It does piles of lowering and things on-the-fly as
>>> > well as creating temporary variables for various things.  The best we
>>> > could
>>> > hope to guarnatee would be that NIR -> SPIR-V -> NIR -> vars_to_ssa ->
>>> > CSE
>>> > is idempotent.  Even that might be a bit of a stretch.
>>> >
>>> >>
>>> >> > One could avoid this by making the original compile always go through
>>> >> > SPIR-V, and just drop glsl_to_nir altogether, so both take the same
>>> >> > paths.  But...it's kind of an unnecessary step in the common case...
>>> >>
>>> >> We may eventually partially do that, but that shouldn't block (any)
>>> >> other work.  In the short term it would likely add compile overhead
>>> >> that
>>> >> many would find unacceptable... by virtue of being non-zero.
>>> >>
>>> >> > Just serializing/reading back the NIR and resuming the compile from
>>> >> > the
>>> >> > exact same IR would also solve that problem.
>>> >> >
>>> >> > Or, just being -really- careful with the translator, I guess...
>>> >> >
>>> >> >> One nice thing about SPIR-V is that all of the handling of uniform
>>> >> >> layouts, initial uniform values, attribute locations, etc. is
>>> >> >> already
>>> >> >> serialized.  If I'm not mistaken, that was one of the big pain
>>> >> >> points
>>> >> >> for all of the existing on-disk storage methods.  All of that has
>>> >> >> been
>>> >> >> sorted out for SPIR-V, and we have to make it work anyway.
>>> >> >
>>> >> > That is pretty nice.  I don't recall it being that painful, but, not
>>> >> > reinventing things is kind of nice too...
>>> >>
>>> >> Maybe the right answer is to share some things from SPIR-V (e.g., the
>>> >> way it describes I/O) to reduce duplication, but serialize NIR
>>> >> instructions and control flow in a "native" format.
>>> >
>>> >
>>> > I strongly suspect that there is some overestimation of how much code
>>> > nir_serialize would actually be here.  I just looked at nir_clone.c and
>>> > it's
>>> > 781 lines.  It could probably drop by 50-100 LOC if we didn't expose
>>> > helpers
>>> > for also cloning of single functions, variables, and constants.  I would
>>> > expect nir_serialize to be able to handle serialization and
>>> > deserialization
>>> > in about 2x the code of nir_clone.c.  By comparison, SPIR-V -> NIR is
>>> > 8159
>>> > LOC and counting (that doesn't include generated anything) and I would
>>> > expect GLSL -> SPIR-V to be similarly sized.
>>>
>>> I just finished typing up a nir_serialize implementation, although I
>>> haven't debugged it yet. It wound up being 1150 lines, including
>>> comments and whatnot:
>>>
>>> https://cgit.freedesktop.org/~cwabbott0/mesa/commit/?h=nir-serialize&id=2bacd646460328940c5021d1bdaced09a45ed947
>>
>>
>> Taking the lock... I've got a test harness written for it and am working on
>> fixing some of the bugs. :-)

Pushed a few things. I randomly picked one test, I've almost got it working now.