The gallium and vulkan drivers deal with vertex attribute emission
differently. The gallium driver re-emits the VS attributes on each
draw, while the vulkan driver uses explicit attribute/image
descriptor dirtiness tracking, and could keep the attribute array
around if a new pipeline using a different number of attribute is
bound. If we want to be able to do that, we need to assign a fixed
offset for image attributes, such that the Vulkan descriptor
lowering pass knows where the images are in the attribute table.
We could teach the Bifrost backend how to deal with a custom offset
but it doing that in a lowering pass also simplifies the Midgard
code.
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Reviewed-by: Mary Guillemard <mary.guillemard@collabora.com>
Acked-by: Erik Faye-Lund <erik.faye-lund@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/28200>
This also fix missing encoding of indice with non immediate index.
Signed-off-by: Mary Guillemard <mary.guillemard@collabora.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27846>
Lower tex/sampler table in indices on panfrost.
This also implement wide indices and change the format of texture and sampler
indices received by the compiler.
Signed-off-by: Mary Guillemard <mary.guillemard@collabora.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27846>
Rework the way we compute thread info to make it mostly GPU-agnostic
outside of the kmod backend.
The new logic is based on the following information extracted from
GPU registers:
- mximum number of threads per core
- maximum number ot threads per workgroup
- number of registers per core
If the GPU doesn't provide this information (registers are zero), we
pick the per-arch defaults we had in panfrost_max_thread_count().
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Reviewed-by: Antonino Maniscalco <antonino.maniscalco@collabora.com>
Reviewed-by: Erik Faye-Lund <erik.faye-lund@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/26358>
Panfrost generally treats 2D multisampled images like 3D images,
with the R coordinate holding the sample index. This commit adds
a lowering pass to convert 2DMS images to 3D for the compiler. It
is not actually invoked yet.
Signed-off-by: Eric R. Smith <eric.smith@collabora.com>
Reviewed-by: Erik Faye-Lund <erik.faye-lund@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27626>
If dual blending is enabled, only 1 output is supported. Multiple
outputs confuse the write combining pass in this case, leading to
incorrect output and/or an assert failure in emit_fragment_store.
The fix is straightforward, just skip the speculative emitting of
multiple outputs in the case where dual source blending is enabled.
This also adds an extra sanity check in `pan_nir_lower_zs_store` to
check for only one blend store being present.
Fixes: c65a9be421 ("panfrost: Preprocess shaders at CSO create time")
Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/9487
Co-Authored-By: Eric R. Smith <eric.smith@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/26474>
Instead, we replace every use of it with nir_def. Most of this commit
was generated by sed:
sed -i -e 's/dest.ssa/def/g' src/**/*.h src/**/*.c src/**/*.cpp
A few manual fixups were required in lima and the nir_legacy code.
Acked-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/24674>
We could add a nir_def_num_components() helper but we use
ssa.num_components about 3x as often as nir_dest_num_components() today
so that's a major Coccinelle refactor anyway and this doesn't make it
much worse. Most of this commit was generated byt the following
semantic patch:
@@
expression D;
@@
<...
-nir_dest_num_components(D)
+D.ssa.num_components
...
Some manual fixup was needed, especially in cpp files where Coccinelle
tends to give up the moment it sees any interesting C++.
Acked-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/24674>
We could add a nir_def_bit_size() helper but we use ->bit_size about 3x
as often as nir_dest_bit_size() today so that's a major Coccinelle
refactor anyway and this doesn't make it much worse. Most of this
commit was generated byt the following semantic patch:
@@
expression D;
@@
<...
-nir_dest_bit_size(D)
+D.ssa.bit_size
...
Some manual fixup was needed, especially in cpp files where Coccinelle
tends to give up the moment it sees any interesting C++.
Acked-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/24674>
Even on Valhall, vertex_id is zero-based in a transform feedback program. Lower
that for transform feedback programs properly since it wouldn't happen
automatically on Valhall. Fixes assertion fails.
Fixes: 91ffd10351 ("pan/bi: Lower gl_VertexID in NIR")
Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/24198>
It really isn't that hard. This drops the roundmode optimization but otherwise
should be at parity to what there was before, and it's massively more competent
at it anyway.
total instructions in shared programs: 1514477 -> 1508444 (-0.40%)
instructions in affected programs: 645848 -> 639815 (-0.93%)
helped: 2712
HURT: 187
Instructions are helped.
total bundles in shared programs: 645069 -> 642999 (-0.32%)
bundles in affected programs: 136233 -> 134163 (-1.52%)
helped: 1242
HURT: 319
Bundles are helped.
total quadwords in shared programs: 1130469 -> 1125969 (-0.40%)
quadwords in affected programs: 379780 -> 375280 (-1.18%)
helped: 1878
HURT: 376
Quadwords are helped.
total registers in shared programs: 90577 -> 90633 (0.06%)
registers in affected programs: 5627 -> 5683 (1.00%)
helped: 309
HURT: 294
Inconclusive result (value mean confidence interval includes 0).
total threads in shared programs: 55594 -> 55607 (0.02%)
threads in affected programs: 118 -> 131 (11.02%)
helped: 43
HURT: 33
Inconclusive result (value mean confidence interval includes 0).
total spills in shared programs: 1399 -> 1371 (-2.00%)
spills in affected programs: 345 -> 317 (-8.12%)
helped: 10
HURT: 4
total fills in shared programs: 5273 -> 5133 (-2.66%)
fills in affected programs: 1035 -> 895 (-13.53%)
helped: 12
HURT: 4
Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
Reviewed-by: Italo Nicola <italonicola@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/23769>
Via Coccinelle patches
@@
expression a, b, c;
@@
-nir_channels(b, a, (1 << c) - 1)
+nir_trim_vector(b, a, c)
@@
expression a, b, c;
@@
-nir_channels(b, a, BITFIELD_MASK(c))
+nir_trim_vector(b, a, c)
@@
expression a, b;
@@
-nir_channels(b, a, 3)
+nir_trim_vector(b, a, 2)
@@
expression a, b;
@@
-nir_channels(b, a, 7)
+nir_trim_vector(b, a, 3)
Plus a fixup for pointless trimming an immediate in RADV and radeonsi.
Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
Reviewed-by: Rhys Perry <pendingchaos02@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/23352>
Every nir_ssa_def is part of a chain of uses, implemented with doubly linked
lists. That means each requires 2 * 64-bit = 16 bytes per def, which is
memory intensive. Together they require 32 bytes per def. Not cool.
To cut that memory use in half, we can combine the two linked lists into a
single use list that contains both regular instruction uses and if-uses. To do
this, we augment the nir_src with a boolean "is_if", and reimplement the
abstract if-uses operations on top of that list. That boolean should fit into
the padding already in nir_src so should not actually affect memory use, and in
the future we sneak it into the bottom bit of a pointer.
However, this creates a new inefficiency: now iterating over regular uses
separate from if-uses is (nominally) more expensive. It turns out virtually
every caller of nir_foreach_if_use(_safe) also calls nir_foreach_use(_safe)
immediately before, so we rewrite most of the callers to instead call a new
single `nir_foreach_use_including_if(_safe)` which predicates the logic based on
`src->is_if`. This should mitigate the performance difference.
There's a bit of churn, but this is largely a mechanical set of changes.
Signed-off-by: Alyssa Rosenzweig <alyssa@collabora.com>
Reviewed-by: Faith Ekstrand <faith.ekstrand@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/22343>
Only the GL driver produces/consumes these, they shouldn't be in the common
shader_info.
Signed-off-by: Alyssa Rosenzweig <alyssa@collabora.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20906>
Drop the backend compiler sysval handling in favour of the pass in the GL
driver, bringing us into compliance with Ekstrand's rule.
Signed-off-by: Alyssa Rosenzweig <alyssa@collabora.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20906>
Do it explicitly in NIR rather than implicitly in the Midgard compiler. This
avoids a nasty sideband input for the render target formats and sample count,
for blend shaders on midgard only.
Signed-off-by: Alyssa Rosenzweig <alyssa@collabora.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20906>
This is a flag-day change to how we compile. We split preprocessing NIR into a
separate step from compiling, giving the driver a chance to apply its own
lowerings on the preprocessed NIR before the final optimization loop. During
that time, the different producers of NIR (panfrost, panvk, blend shaders, blit
shaders...) will be able to (differently) lower system values.
Signed-off-by: Alyssa Rosenzweig <alyssa@collabora.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20906>
Removes a lot of indentation, and improves metadata handling.
Signed-off-by: Alyssa Rosenzweig <alyssa@collabora.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20906>
It doesn't make sense for shader stages other than fragment (and blend which is
fragment-like), assert this.
Signed-off-by: Alyssa Rosenzweig <alyssa@collabora.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20906>
This sideband input is now unused, as the information is available locally
within the NIR as it should be.
Signed-off-by: Alyssa Rosenzweig <alyssa@collabora.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20906>
See previous commits for justification. Later, we'll split up NIR processing in
a few steps to give the caller a chance to lower the sysval, at which point the
goofy inputs here will go away.
v2: Only lower in fragment shaders. Likely harmless to run elsewhere but still
wrong because the location enum is defined per-stage.
Signed-off-by: Alyssa Rosenzweig <alyssa@collabora.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com> [v1]
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20906>
This uses the new NIR sysvals to avoid materializing magic sysvals in the
driver, getting us closer to the Ekstrand Rule.
v2: Only lower for fragment shaders. Lowering in vertex shaders should be a
no-op, except that FRAG_RESULT_SAMPLE_MASK shadows a VARYING_SLOT for fog
coords, causing v1 of this patch to regress fog. Caught by the G52 piglit job in
CI. Thank you, Marge.
Signed-off-by: Alyssa Rosenzweig <alyssa@collabora.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com> [v1]
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20906>
This turns the early pass into a late pass, which is important because it
depends on the shader key and therefore should be called by the driver instead
of the compiler preprocessing. It's also simpler this way.
The shader key work is waiting for review in another merge request. In the mean
time, this patch will let us run blend lowering early for blend shaders on
Midgard.
Signed-off-by: Alyssa Rosenzweig <alyssa@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20836>
This is a form of lowered I/O, it needs I/O semantics so we can know the
location to store to instead of passing via a sideband.
Over in !20906, we will use the BASE to lower blend shader with multisampling in
NIR instead of passing the number of samples and framebuffer format along a
sideband to the Midgard compiler. That's not needed for this series (this patch
was cherry-picked to avoid regressions in the lower_blend changes) but it's good
to model the full form of the I/O lowered intrinsic here.
Signed-off-by: Alyssa Rosenzweig <alyssa@collabora.com>
Reviewed-by: Italo Nicola <italonicola@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20836>
This switches us over to Mesa's code style [1], normalizing us within the tree.
The results aren't perfect, but they bring us a hell of a lot closer to the rest
of the tree. Panfrost doesn't feel so foreign relative to Mesa with this, which
I think (in retrospect after a bunch of years of being "different") is the right
call.
I skipped PanVK because that's paused right now.
find panfrost/ -type f -name '*.h' | grep -v vulkan | xargs clang-format -i;
find panfrost/ -type f -name '*.c' | grep -v vulkan | xargs clang-format -i;
clang-format -i gallium/drivers/panfrost/*.c gallium/drivers/panfrost/*.h ; find
panfrost/ -type f -name '*.cpp' | grep -v vulkan | xargs clang-format -i
[1] https://docs.mesa3d.org/codingstyle.html
Signed-off-by: Alyssa Rosenzweig <alyssa@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20425>
If nir->info.shared_size = 0 but grid->variable_shared_mem > 0, the shader uses
shared memory but the compiler may not realize that. We need to disable
workgroup merging even in this case. The alternate approach is to statically
check for shared intrinsics in the compiler, but this is a bit easier all things
considered.
Signed-off-by: Alyssa Rosenzweig <alyssa@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/18581>
NIR deemphasizes nir_variable. We want to transition off it. Instead of walking
the list of variables and playing games with the GLSL types to collect varying
information, walk the list of instructions and use the I/O semantics to collect
similar information.
In addition to avoiding the reliance on nir_variable, this fixes handling of
struct varyings under certain circumstances. Such programs are compiled by the
GLES3.1 CTS but not used, so without this fix, the affected tests would regress
when precompiling.
Signed-off-by: Alyssa Rosenzweig <alyssa@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/19363>