Having this pointer in the key is undesirable since it makes
copying keys difficult and error prone (as seen in previous
patches), also, it is only there for convenience and we don't
strictly need it (in fact the vulkan driver doesn't use it at
all), so let's just get rid of it so our v3d_key is fully
static.
Reviewed-by: Juan A. Suarez <jasuarez@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/25418>
The RAM shader cache was using the v3d_key for hashes and
comparisons which is not correct. Particularly, this struct
has a void pointer where we store a reference to an uncompiled
shader with the NIR code, and that is of course not accounted
for when hashing and comparing keys, which can lead to bogus
cache hits.
This patch introduces a v3d_cache_key that has both the v3d key
and a sha1 of the uncompiled NIR. Now key hashing and comparison
is done on the static part of the v3d key (that is, excluding the
uncompiled shader pointer, which may be invalid in the cache if
the original shader was deleted) and taking the sha1 from the
uncompiled shader. This also makes sure the shader key we store
in the cache has a NULL shader_state pointer to make it more
clear that this field may not be used at all for caching purposes.
This fixes GPU hangs with some OpenCL tests (through Rusticl)
caused by incorrect RAM cache hits.
Reviewed-by: Juan A. Suarez <jasuarez@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/25418>
Our shader key includes a void pointer that we can't just memcmp,
so add helpers that allow us toget the 'static' portion and size
of a key. We will use this to fix up the shader cache in v3d in
a later patch.
Reviewed-by: Juan A. Suarez <jasuarez@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/25418>
This is to inform you of some planned downtime in the LAVA lab as follows:
* Start: 2023-10-02 08:00 BST (07:00 UTC)
* End: 2023-10-02 12:00 BST (11:00 UTC)
Signed-off-by: Sergi Blanch Torne <sergi.blanch.torne@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/25368>
To reduce shader variants, the tilebuffer lowering code does not know the
actual texture targets of the spilled render targets, only whether they are
layered or not. As such, all layered targets (3D, cube map, etc) get written out
uniformly as 2D Arrays. For that to work, the driver needs to do the
corresponding transform.
Regular imageStore() instructions are not affected by any of this.
Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
Do not force a single-layer view, use an actual array attachment when there are
multiple layers, since this corresponds to a layered framebuffer that will write
to an array with the eMRT path.
Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
We can't technically expose the extension without a higher GL version, but the
implemented subset should work and this lets us test with piglit.
Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
If we spill render targets with a layered framebuffer, our spilled targets are
assumed to be 2D Arrays (in general). We need to use arrayed image operations to
load/store from these. The layer is given by the layer as read in the fragemnt
shader. This handles the eMRT portion of layered rendering.
Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
Just add a flag for it. We don't care about the actual # of layers when
calculating the layout, only the boolean fact of being layered or not. The
reason we need this at all is because the eMRT implementation needs to
account for layering and that is only keyed off the tilebuffer layout.
Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
The hardware needs the layer ID and the viewport index packed together. That
consumes an entire varying slot, if we want those available in the frag shader
we need a separate slot. Add a pass to insert the extra packed write.
Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
We'll implement layer ID reads in the frag shader with a varying read, but if
the VS doesn't write the varying we need to return 0 per the spec. Add a sysval
to detect that case so we can handle it at runtime without keys.
Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
With the exception of the backwards branch for loops, all the control flow we
insert during instruction selection just predicates instructions rather than
actually jumping around. That means, for example, we execute both sides of the
if even for a uniform condition! That's inefficient. The solution is insert
jmp_exec_none instructions after control flow in order to skip unexecuted
regions, which is much faster than predicating them out. However, jmp_exec_none
is costly in itself, so we need to use a heuristic to determine when it's
actually beneficial.
This uses a very simple heuristic for this purpose. However, it is a massive
performance speed-up for Dolphin uber shaders: 39fps -> 67fps at 2x resolution.
Nearly a doubling of performance!
Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
jmp_exec_none variant that jumps to the last instruction of the target block,
rather than the beginning. This is convenient for skipping over elses, while
still executing the block-final pop_exec instruction. Similarly for skipping
over loop bodies while still executing the block-final pop_exec, after break
instructions.
Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
Add an optional pointer to a target block for these instructions. This does NOT
act like a logical branch, and does NOT get added to the logical control flow.
It is ignored wholesale until after RA, when physical edges may be inserted by a
pass we add later in this series.
Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
So we get adequate testing of continues, rather than lowering them in GLSL. We
don't really /want/ to see continues but lowering them away will just make them
harder to test... and besides, we should be optimizing them in NIR (not GLSL) so
we can get the win on Vulkan too.
Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
Although this is well-motivated, perf effect seems to be neglible for Dolphin.
It does prevent the scheduler from making things worse by sinking these
instructions though, so as a way to prevent future problems this seems sensible.
The kind of problem this affects (late discard) isn't modelled in shader-db.
Nevertheless, nothing concerning there:
total instructions in shared programs: 1756699 -> 1756722 (<.01%)
instructions in affected programs: 10106 -> 10129 (0.23%)
helped: 21
HURT: 41
Inconclusive result (value mean confidence interval includes 0).
total bytes in shared programs: 11525404 -> 11525452 (<.01%)
bytes in affected programs: 72900 -> 72948 (0.07%)
helped: 27
HURT: 41
Inconclusive result (value mean confidence interval includes 0).
total halfregs in shared programs: 483394 -> 483286 (-0.02%)
halfregs in affected programs: 4945 -> 4837 (-2.18%)
helped: 88
HURT: 78
Inconclusive result (value mean confidence interval includes 0).
Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
This fixes live range splitting with 3D textureGrad(), which involves vectors
larger than the natural 128-bit maximum and hence requires special handling.
Fixes this assert with a combination of debug flags and new patches:
unsigned int find_best_region_to_evict(struct ra_ctx *, unsigned int,
unsigned int *, unsigned int *):
Assertion `(rctx->bound % size) == 0 && "register file size must be aligned
to the maximum vector size"' failed
Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
This makes for cleaner agxdecodes, I think this matches what I've seen on the
macOS side but I might be misremembering. Certainly shouldn't hurt.
This only applies for direct draws.
Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
In particular, removes the dependency of r600_formats.h on r600_pipe.h so
it can be shared between Gallium and Vulkan.
Reviewed-by: Gert Wollny <gert.wollny@collabora.com>
Signed-off-by: Vitaliy Triang3l Kuzmin <triang3l@yandex.ru>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/24513>
PAL doesn't do it. If drivers want IBs not to share cache lines with other buffers,
they should align the size manually.
Reviewed-by: Timur Kristóf <timur.kristof@gmail.com>
Reviewed-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/25492>
