<html>
<head>
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<p><a class="moz-txt-link-freetext" href="https://gitlab.freedesktop.org/mesa/mesa/merge_requests/178">https://gitlab.freedesktop.org/mesa/mesa/merge_requests/178</a></p>
<div class="wiki">
<p dir="auto">TL; DR; This series implements all the mesa bits
needed to enable
ARB_gl_spirv and ARB_spirv_extensions on the i965 driver, and
with
that, expose OpenGL 4.6 on that driver.</p>
<p dir="auto">Detailed explanation: we reached a point where we
consider our
development branch good enough to enable both extensions on
i965, so
we preferred to send the full series, instead of keeping sending
subseries of specific sub-features. As a collateral effect of
enabling
both extensions we can also expose OpenGL 4.6 on i965.</p>
<p dir="auto">It is worth to note that some of those patches could
be easily
squashed with some others, but we preferred to keep them, to
make
easier the review process.</p>
<p dir="auto">Although on this MR the patch numbering will get
quickly obsolete,
here an overview of the the patches at the moment this MR got
sent:</p>
<ul dir="auto">
<li>
<p>Patches 1-5 provides improvements over current ARB_gl_spirv
xfb
support. It basically extends the xfb nir gathering pass
recently
added for Vulkan needs, and then replaces the ARB_gl_spirv
custom
one and uses it instead.</p>
</li>
<li>
<p>Patches 6-24 implemens UBO/SSBO support, and calls to link
them on
the i965 driver. It is worth to note that it is using the
new deref
based UBO/SSBO path, so we could get arrays of arrays
properly
supported on the spirv to nir pass.</p>
</li>
<li>
<p>Patches 25-33 adds the different resources already
supported by the
ARB_gl_spirv linker to the resource list (input/output,
ubo/ssbo,
etc), and fill up properly some data for such resources, in
preparation to the program interface queries support.</p>
</li>
<li>
<p>Patches 34-41 fixes and extends several program interfaces
queries. In most of the cases they are just to handling the
fact
that now resource name can be NULL, and applies the value
expected
for such case defined on the ARB_gl_spirv spec.</p>
</li>
<li>
<p>Patches 42-45 gets serialization working when using SPIR-V
shaders. As the previous case, most patches are about
handling the
fact that names can be NULL, plus initializing
UniformDataDefaults
when using the ARB_gl_spirv nir linker. All this got
interaction
with ARB_get_program_binary working (at least with the tests
we had
an hand).</p>
</li>
<li>
<p>Patch 46 prevents to load the shader cache. Even if we
handle the
serialization with previous patches, shader cache is still
not
working with SPIR-V shaders, so we prevent the cache to be
used on
that case. Taking into account that the ARB_gl_spirv spec
doesn't
require the cache to be working, we thought that we
shouldn't wait
to enable the extension until finishing it, and clearly we
shouldn't wait for it to send to review.</p>
</li>
<li>
<p>Patches 47-48 add some validations. It is worth to note
that
ARB_gl_spirv doesn't require too much validation. That
includes the
validations added with those patches. As with Vulkan, it is
assumed
that the SPIR-V shaders should be correct. Having said so,
the spec
also allows to raise them as link errors if you want. We
implemented those validations during development, and we
think that
they are fine and simple enough validations to be included.</p>
</li>
<li>
<p>Patch 49: enable ARB_gl_spirv on i965.</p>
</li>
<li>
<p>Patches 50-55 adds the support for ARB_spirv_extensions,
and enable
it on i965. FWIW, we sent this several months ago to review.
It
didn't change anything since then (except minor changes due
rebases).</p>
</li>
<li>
<p>Patch 56: exposes OpenGL 4.6 on i965.</p>
</li>
</ul>
<p dir="auto">So about how this series got tested. First, this
series gets all the
ARB_gl_spirv and ARB_spirv_extensions CTS tests passing. So in
theory
that should be enough to enable both extensions. In the
practice,
those tests were not enough, so we tested this series with
piglit
patches. For that we had two types of tests:</p>
<ul dir="auto">
<li>
<p>Barebone tests: tests we were adding as we implemented the
extension. Piglit master already have some of them.</p>
</li>
<li>
<p>Borrowed tests: recycled tests from other specs. For this
we use a
heavily modified version of a script wrote by Nicolai
Hähnle, that
do some fixes on shader_test tests, and then uses glslang to
convert them to SPIR-V.</p>
</li>
</ul>
<p dir="auto">Unfourtunately, right now the previous two are not
ready for review
(except those that we already sent), as we were focusing on
getting
the Mesa support done. We are cleaning both this week, so we
will send
to review all those on the following weeks.</p>
<p dir="auto">Having said so, right now we get the following
outcome when run all
the shader piglit tests on SPIR-V mode:</p>
<p dir="auto">[34596/34596] skip: 6020, pass: 28559, fail: 17,
crash: 0</p>
<p dir="auto">FWIW, this is the outcome on GLSL mode:</p>
<p dir="auto">[34596/34596] skip: 4877, pass: 29714, fail: 3,
crash: 2</p>
<p dir="auto">Probably the main difference is the greater amount
of skipped tests on
the SPIR-V run (for example, those crashed tests on the GLSL run
are
skipped on the SPIR-V run). This increase comes from:</p>
<ul dir="auto">
<li>
<p>Tests that doesn't make sense on ARB_gl_spirv, like old
GLSL
functionality that is not supported, or name-based
validations.</p>
</li>
<li>
<p>Glslang limitations when dealing with some shaders, usually
when
auto-assigning locations/bindings through stages. Note that
for
the latter, it is assumed that your GLSL shaders would set
explicitly the location/bindings. glslang provide that
functionality somewhat experimental, as a way to support
already
written old shaders (like we have).</p>
</li>
</ul>
<p dir="auto">For both we try to automatize the skipping as much
as possible (on the
mentioned script) but we also maintain a exclude list for them.</p>
</div>
</body>
</html>