<div dir="ltr"><div><div>Taking the "just reply to Nicolai" approach like everyeone else...<br><br></div>Nicolai,<br><br></div>First off, thanks for working on this!  I've given it a bit of thought in the context of i965, so I've got a few ideas about how you could overcome some of the issues.<br><div><div><div><div class="gmail_extra"><br><div class="gmail_quote">On Sun, May 21, 2017 at 3:48 AM, Nicolai Hähnle <span dir="ltr"><<a href="mailto:nhaehnle@gmail.com" target="_blank">nhaehnle@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi all,<br>
<br>
I've been looking into ARB_gl_spirv for radeonsi. I don't fancy re-inventing the ~8k LOC of src/compiler/spirv, and there's already a perfectly fine SPIR-V -> NIR -> LLVM compiler pipeline in radv, so I looked into re-using that.<br>
<br>
It's not entirely straightforward because radeonsi and radv use different "ABIs" for their shaders, i.e. prolog/epilog shader parts, different user SGPR allocations, descriptor loads work differently (obviously), and so on.<br></blockquote><div><br></div><div>We have this same problem with NIR coming into our back-end compiler from Vulkan, BLORP, and GLSL where they all speak very slightly different variants.  The approach we've taken for most of this is to add front-end-specific lowering passes that turn it into a common "ABI" that is consumed by our two back-ends.  You could also do some switching in NIR -> LLVM and that should work fine for small things.  If you want to see what we've done, look for any anv_nir_*.c or brw_nir_*.c in src/intel/vulkan or src/mesa/drivers/dri/i965.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Still, it's possible to separate the ABI from the meat of the NIR -> LLVM translation. So here goes...<br>
<br>
<br>
The Step-by-Step Plan<br>
=====================<br>
<br>
1. Add an optional GLSL-to-NIR path (controlled by R600_DEBUG=nir) for very simple VS-PS pipelines.<br></blockquote><div><br></div><div>This sounds like both a good step and something that I think would be neat to see.  There have been a variety of different discussions over the last few years about compiler design choices but we've lacked the ability to get any good apples-to-apples comparisons.  This may provide some opportunities to do so.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
2. Add GL_ARB_gl_spirv support to Mesa and test it on simple VS-PS pipelines.<br>
<br>
3. Fill in all the rest:<br>
3a. GL 4.x shader extensions (SSBOs, images, atomics, ...)<br>
3b. Geometry and tessellation shaders<br>
3c. Compute shaders<br>
3d. Tests<br></blockquote><div><br></div><div>spirv_to_nir should handle almost everything here (with the possible exception of stand-alone atomic counters).<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I've started with step 1 and got basic GLSL 1.30-level vertex shaders working via NIR. The code is here: <a href="https://cgit.freedesktop.org/~nh/mesa/log/?h=nir" rel="noreferrer" target="_blank">https://cgit.freedesktop.org/~<wbr>nh/mesa/log/?h=nir</a><br>
<br>
The basic approach is to introduce `struct ac_shader_abi' to capture the differences between radeonsi and radv. In the end, the entry point for NIR -> LLVM translation will simply be:<br>
<br>
   void ac_nir_translate(struct ac_llvm_context *ac,<br>
                         struct ac_shader_abi *abi,<br>
                         struct nir_shader *nir);<br>
<br>
Setting up the LLVM function with its parameters is still considered part of the driver.<br>
<br>
<br>
Questions<br>
=========<br>
<br>
1. How do we get good test coverage?<br>
------------------------------<wbr>------<br>
A natural candidate would be to add a SPIR-V execution mode for the piglit shader_runner. That is, use build scripts to extract shaders from shader_test files and feed them through glslang to get spv files, and then load those from shader_runner if a `-spirv' flag is passed on the command line.<br>
<br>
This immediately runs into the difficulty that GL_ARB_gl_spirv wants SSO linking semantics, and I'm pretty sure the majority of shader_test files don't support that -- if only because they don't set a location on the fragment shader color output.<br>
<br>
Some ideas:<br>
1. Add a GL_MESA_spirv_link_by_name extension<br>
2. Have glslang add the locations for us (probably difficult because glslang seems to be focused on one shader stage at a time.)<br>
3. Hack something together in the shader_test-to-spv build scripts via regular expressions (and now we have two problems? :-) )<br>
4. Other ideas?<br></blockquote><div><br></div><div>Another thing I've considered when thinking about doing this myself would be to add a SPIR-V back-end to either NIR or GLSL IR.  Then you can hook it up as GLSL -> SPIR-V -> NIR and run some tests.  If you did this post-linking, then you could just use the locations assigned by the linker and all of the tests should basically work.  I'm not sure about whether doing NIR -> SPIR-V or GLSL -> SPIR-V would be better.  Being able to serialize NIR has its uses but, if one wanted to make the stand-alone compiler into a GLSLang competitor, I think going directly from GLSL IR would be better. <br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
2. What's the Gallium interface?<br>
------------------------------<wbr>--<br>
Specifically, does it pass SPIR-V or NIR?<br>
<br>
I'm leaning towards NIR, because then specialization, mapping of uniform locations, atomics, etc. can be done entirely in st/mesa.<br>
<br>
On the other hand, Pierre Moreau's work passes SPIR-V directly. On the third hand, it wouldn't be the first time that clover does things differently.<br>
<br>
<br>
3. NIR vs. TGSI<br>
---------------<br>
It is *not* a goal for this project to use NIR for normal GLSL shaders. We'll keep the TGSI backend at least for now. But it makes sense to think ahead.<br>
<br>
A minor disadvantage of NIR is that the GLSL-to-NIR path is not as solid as the GLSL-to-TGSI path yet, but this shouldn't be too difficult to overcome.<br>
<br>
The major disadvantage of NIR is that it doesn't have serialization. radeonsi uses the fact that TGSI *is* a serialization format for two things:<br>
<br>
- The internal shader cache, which avoids re-compiling the same shader over and over again when it's linked into different programs. (This part only needs a strong hash.)<br>
<br>
- The (disk) shader cache stores the TGSI so that it's available in case additional shader variants need to be compiled on the fly.<br>
<br>
Some ideas:<br>
<br>
1. Add a serialization format for NIR. This is the most straight-forward solution, but it's a lot of work for a comparatively small feature.<br></blockquote><div><br></div><div>This shouldn't be hard to do.  Some instruction types like texture instructions will be a bit of work (not bad) but most of the instructions are ALU instructions or intrinsics and those have a very nice introspection mechanism that would let you write a fairly small piece of code that can handle anthing.  All told, I doubt a well-written NIR [de]serialization pass would take much more than around 1k LOC.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
1b. Use SPIR-V as a serialization format for NIR. This is more work for serialization than a custom format due to the ceremony involved in SPIR-V, but we already have deserialization. Also, it'd implicitly give us an alternative GLSL-to-SPIR-V compiler, which is kind of neat.<br></blockquote><div><br></div><div>Giving it a fairly small quantity of thought, I think this would probably work out ok.  The biggest problem would be that NIR is typeless so you would have to detect type changes and insert bitcast instructions at various places.  There are some other mismatches but those are mostly things that NIR expects to be lowered so they only show up in SPIR-V -> NIR.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
2. Don't store TGSI/NIR in the (disk) shader cache. The reason we have to do that right now is that radeonsi does multi-threaded compilation, and so we cannot fallback all the way back to GLSL compilation if we need to compile a new shader variant. However, once we properly implement ARB_parallel_shader_compile, this issue will go away.<br>
<br>
This doesn't address the internal shader cache, though.<br>
<br>
3. Have st/mesa recognize when the same shader is linked into multiple programs and avoid generating duplicate shader CSOs where possible. This is non-trivial mostly because linking can map shader I/O into different places, but I imagine that it would cover the majority of the cases caught by radeonsi's internal shader cache.<br>
<br>
4. Something else for the internal shader cache?<br>
<br>
5. Use TGSI after all. TGSI really isn't such a bad format. It does have some warts, like pretending that everything is a vec4, which means that f64 support is already annoying, f16 support is going to be annoying, the way we do UBOs should probably be re-written (since it cannot support std430 packing, which would be nice to have), and real (non-inline) function support will be nasty if we ever get there. But TGSI works, it's patient and straightforward.<br>
<br>
That said, NIR is nicer in several ways. Not using it just because it can't do serialization would be sad, not to mention those ~8k LOCs of SPIRV-to-NIR. We could go SPIRV-to-NIR-to-TGSI, of course, but that's not exactly great for compiler performance.<br>
<br>
All of this doesn't necessarily block the project of adding GL_ARB_gl_spirv, but it'd be nice to think a bit ahead.<br>
<br>
So, what does everybody think? I'm particularly interested in the nouveau folks' take on the whole NIR vs. TGSI thing, and any ideas on how to address the above questions.<br>
<br>
Cheers,<br>
Nicolai<span class="HOEnZb"><font color="#888888"><br>
-- <br>
Lerne, wie die Welt wirklich ist,<br>
Aber vergiss niemals, wie sie sein sollte.<br>
______________________________<wbr>_________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/<wbr>mailman/listinfo/mesa-dev</a><br>
</font></span></blockquote></div><br></div></div></div></div></div>