<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Sep 12, 2017 at 11:09 AM, Jason Ekstrand <span dir="ltr"><<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5">On Tue, Sep 12, 2017 at 10:12 AM, Ian Romanick <span dir="ltr"><<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="m_5144317446975850872HOEnZb"><div class="m_5144317446975850872h5">On 09/11/2017 11:17 PM, Kenneth Graunke wrote:<br>
> On Monday, September 11, 2017 9:23:05 PM PDT Ian Romanick wrote:<br>
>> On 09/08/2017 01:59 AM, Kenneth Graunke wrote:<br>
>>> On Thursday, September 7, 2017 4:26:04 PM PDT Jordan Justen wrote:<br>
>>>> On 2017-09-06 14:12:41, Daniel Schürmann wrote:<br>
>>>>> Hello together!<br>
>>>>> Recently, we had a small discussion (off the list) about the NIR<br>
>>>>> serialization, which was previously discussed in [RFC] ARB_gl_spirv and<br>
>>>>> NIR backend for radeonsi.<br>
>>>>><br>
>>>>> As this topic could be interesting to more people, I would like to<br>
>>>>> share, what was talked about so far (You might want to read from bottom up).<br>
>>>>><br>
>>>>> TL;DR:<br>
>>>>> - NIR serialization is in demand for shader cache<br>
>>>>> - could be done either directly (NIR binary form) or via SPIR-V<br>
>>>>> - Ian et al. are working on GLSL IR -> SPIR-V transformation, which<br>
>>>>> could be adapted for a NIR -> SPIR-V pass<br>
>>>>> - in NIR representation, some type information is lost<br>
>>>>> - thus, a serialization via SPIR-V could NOT be a glslang alternative<br>
>>>>> (otoh, the GLSL IR->SPIR-V pass could), but only for spirv-opt (if the<br>
>>>>> output is valid SPIR-V)<br>
>>>><br>
>>>> Ian,<br>
>>>><br>
>>>> Tim was suggesting that we might look at serializing nir for the i965<br>
>>>> shader cache. Based on this email, it sounds like serialized nir would<br>
>>>> not be enough for the shader cache as some GLSL type info would be<br>
>>>> lost. It sounds like GLSL IR => SPIR-V would be good enough. Is that<br>
>>>> right?<br>
>>>><br>
>>>> I don't think we have a strict requirement for the GLSL IR => SPIR-V<br>
>>>> path for GL 4.6, right? So, this is more of a 'nice-to-have'?<br>
>>>><br>
>>>> I'm not sure we'd want to make i965 shader cache depend on a<br>
>>>> nice-to-have feature. (Unless we're pretty sure it'll be available<br>
>>>> soon.)<br>
>>>><br>
>>>> But, it would be nice to not have to fallback to compiling the GLSL<br>
>>>> for i965 shader cache, so it would be worth waiting a little bit to be<br>
>>>> able to rely on a SPIR-V serialization of the GLSL IR.<br>
>>>><br>
>>>> What do you suggest?<br>
>>>><br>
>>>> -Jordan<br>
>>><br>
>>> We shouldn't use SPIR-V for the shader cache.<br>
>>><br>
>>> The compilation process for GLSL is: GLSL -> GLSL IR -> NIR -> i965 IRs.<br>
>>> Storing the content at one of those points, and later loading it and<br>
>>> resuming the normal compilation process from that point...that's totally<br>
>>> reasonable.<br>
>>><br>
>>> Having a fallback for "some things in the cache but not all the variants<br>
>>> we needed" suddenly take a different compilation pipeline, i.e. SPIR-V<br>
>>> -> NIR -> ... seems risky.  It's a different compilation path that we<br>
>>> don't normally use.  And one you'd only hit in limited circumstances.<br>
>>> There's a lot of potential for really obscure bugs.<br>
>><br>
>> Since we're going to expose exactly that path for GL_ARB_spirv / OpenGL<br>
>> 4.6, we'd better make sure it works always.  Right?<br>
><br>
> In addition to the old pipeline:<br>
><br>
> - GLSL from the app -> GLSL IR -> NIR -> i965 IR<br>
><br>
> GL_ARB_spirv and OpenGL 4.6 add a second pipeline:<br>
><br>
> - SPIR-V from the app -> NIR -> i965 IR<br>
><br>
> Both of those absolutely have to work.  But these:<br>
><br>
> - GLSL -> GLSL IR -> NIR -> SPIR-V -> NIR -> i965 IRs<br>
> - GLSL -> GLSL IR -> SPIR-V -> NIR -> i965 IRs<br>
><br>
> aren't required to work, or even be supported.  It makes a lot of sense<br>
> to support them - both for testing purposes, and as an alternative to<br>
> glslang, for a broader tooling ecosystem.<br>
><br>
> The thing that concerns me is that if you use SPIR-V for the cache, you<br>
> need these paths to not just work, but be _indistinguishable_ from one<br>
> another:<br>
><br>
> - GLSL -> GLSL IR -> NIR -> ...<br>
> - GLSL -> GLSL IR -> NIR -> SPIR-V, then SPIR-V -> NIR -> ...<br>
><br>
> Otherwise the original compile and partially-cached recompile might have<br>
> different properties.  For example, if the the SPIR-V step messes with<br>
> variables or instruction ordering a little, it could trip up the loop<br>
> unroller so the original compiler gets unrolled, and the recompile from<br>
> partial cache doesn't get unrolled.  I don't want to have to debug that.<br>
<br>
</div></div>That is a very compelling argument.  If we want Mesa to be an<br>
alternative to glslang, I think we would like to have that property, but<br>
it's not a hard requirement for that use case.<span><br></span></blockquote><div><br></div></div></div><div>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.<br></div></div></div></div></blockquote><div><br></div><div>I was talking to Jordan about this in person this morning and one other roadblock for using SPIR-V cropped up.  The place where we really want to cache the NIR for highest effectiveness would be right after calling brw_create_nir in brw_link_shader.  At this point in the process, a lot of lowering has been done to NIR intrinsics which have no SPIR-V equivalent.  If we wanted to serialize the NIR as SPIR-V, we would either have to do it much higher up in the pipeline and lose quite a bit of the benefit, or add a NIR extension to SPIR-V that provides those extra intrinsics.  Adding such a SPIR-V extension and relevant headers and to/from NIR code is probably at least 2x the code of writing a nir_serialize pass. <br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div></div><span class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>
> One could avoid this by making the original compile always go through<br>
> SPIR-V, and just drop glsl_to_nir altogether, so both take the same<br>
> paths.  But...it's kind of an unnecessary step in the common case...<br>
<br>
</span>We may eventually partially do that, but that shouldn't block (any)<br>
other work.  In the short term it would likely add compile overhead that<br>
many would find unacceptable... by virtue of being non-zero.<br>
<span><br>
> Just serializing/reading back the NIR and resuming the compile from the<br>
> exact same IR would also solve that problem.<br>
><br>
> Or, just being -really- careful with the translator, I guess...<br>
><br>
>> One nice thing about SPIR-V is that all of the handling of uniform<br>
>> layouts, initial uniform values, attribute locations, etc. is already<br>
>> serialized.  If I'm not mistaken, that was one of the big pain points<br>
>> for all of the existing on-disk storage methods.  All of that has been<br>
>> sorted out for SPIR-V, and we have to make it work anyway.<br>
><br>
> That is pretty nice.  I don't recall it being that painful, but, not<br>
> reinventing things is kind of nice too...<br>
<br>
</span>Maybe the right answer is to share some things from SPIR-V (e.g., the<br>
way it describes I/O) to reduce duplication, but serialize NIR<br>
instructions and control flow in a "native" format.<br></blockquote></span></div></div><div class="gmail_extra"><br></div><div class="gmail_extra">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.</div><span class="HOEnZb"><font color="#888888"><div class="gmail_extra"><br></div><div class="gmail_extra">--Jason<br></div></font></span></div>
</blockquote></div><br></div></div>