There is a small gap of time where the currently bound uncompiled
shaders, and compiled shader variant, are out of sync. Specifically,
between pipe->bind_*_state() and the next draw.
Currently, shaders variants live entirely within a single context,
and when deleting an iris_uncompiled_shader, we check if any of its
variants are currently bound, and defer deleting those until the next
iris_update_compiled_shaders() hook runs and binds new shaders to
replace them. (This is due to the time gap between binding new
uncompiled shaders, and updating variants at draw time when we have
the required NOS in place.)
This works pretty well in a single context world. But as we move to
share compiled shader variants across multiple contexts, it breaks down.
When deleting a shader, we can't look at all contexts to see if its
variants are bound anywhere. We can't even quantify whether those
contexts will run a future draw any time soon, to update and unbind.
One fairly crazy solution would be to delete the variants anyway, and
leave the stale pointers to dead variants in place. This requires
removing any code that compares old and new variants. Today, we do
that sometimes for seeing if the old/new shaders toggled some feature.
Worse than that, though, we don't just have to avoid dereferences, we'd
have to avoid pointer comparisons. If we free a variant, and quickly
allocate a new variant, malloc may return the same pointer. If it's
for the same shader stage, we may get a new different program that has
the same pointer as a previously bound stale one, causing us to think
nothing had changed when we really needed to do updates. Again, this
is doable, but leaves the code fragile - we'd have to guard against
future patches adding such checks back in.
So, don't do that. Instead, do basic reference counting. When a
variant is bound in a context, up the reference. When it's unbound,
decrement it. When it hits zero, we know it's not bound anywhere and
is safe to delete, with no stale references. This ends up being
reasonably cheap anyway, since the atomic is usually uncontested.
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7668>
I have never used this to debug anything in iris, and it's been years
since I even thought about using i965's similar functionality. I'm
planning to move a bunch of shaders out of the global hash table, at
which point it'll be much less useful. So, just drop it.
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8634>
This tried to de-duplicate identical copies of the same shader
assembly, but in the least efficient way possible: it did a linear
walk through every shader in the entire context memcmp'ing the
final assembly (after going through the effort to compile it).
In the end, all it saved was space and number of BOs, not even
state changes.
This optimization has been mostly replaced by st/mesa's cache
mechanism, which looks for multiple shaders that compile to the
same NIR and go further than this did, and actually reuse the
same pipe shader state. That's even more efficient than this.
This seems to still trigger some times, because the NIR that
st/mesa hashes hasn't quite been finalized and stripped. But
it would be better to improve that, not this.
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8634>
In 536727c465, we switched iris to patching the constant data address
into the shader but, thanks to my lack of understanding how iris works,
I got the calculation wrong. I didn't realize, we needed to call
iris_bo_offset_from_base_address to get the BO offset from the start of
instruction state base address.
Fixes: 536727c465 "iris: Patch constant data pointers into shaders"
Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/3596
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6992>
Shader instructions which use UIP/JIP now get formatted with a label
in addition with immediate value, labels have "LABEL%d" format.
v2: - Consider brw_jump_scale when calculating label's offset
From: "Lonnberg, Toni" <toni.lonnberg@intel.com>
Signed-off-by: Danylo Piliaiev <danylo.piliaiev@globallogic.com>
Reviewed-by: Sagar Ghuge <sagar.ghuge@intel.com>
Reviewed-by: Matt Turner <mattst88@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4245>
We were space-leaking iris_compiled_shader objects, leaving them around
basically forever - long after the associated iris_uncompiled_shader was
deleted. Perhaps even more importantly, this left the BO containing the
assembly referenced, meaning those were never reclaimed either. For
long running applications, this can leak quite a bit of memory.
Now, when freeing iris_uncompiled_shader, we hunt down any associated
iris_compiled_shader objects and pitch those (and their BO) as well.
One issue is that the shader variants can still be bound, because we
haven't done a draw that updates the compiled shaders yet. This can
cause issues because state changes want to look at the old program to
know what to flag dirty. It's a bit tricky to get right, so instead
we defer variant deletion until the shaders are properly unbound, by
stashing them on a "dead" list and tidying that each time we try and
delete some shader variants.
This ensures long running programs delete their shaders eventually.
Fixes: ed4ffb9715 ("iris: rework program cache interface")
Reviewed-by: Matt Turner <mattst88@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6075>
Probably the most annoying patch to review from the whole series --
Mark every buffer object use as accessed through some caching domain
with the sequence number of the current synchronization section of the
batch. The additional argument of iris_use_pinned_bo() makes sure I'd
have gotten a compile error if I had missed any buffer added to the
batch validation list.
There are only a few exceptions where a buffer is left untracked while
adding it to the validation list, justified below:
- Batch buffers: These are strictly read-only for the moment.
- BLORP buffer objects: Their seqnos are bumped manually at the end
of iris_blorp_exec() instead, in order to avoid plumbing domain
information through BLORP address combining.
- Scratch buffers: The contents of these are strictly thread-local.
- Shader images and SSBOs: Accesses of these buffers are explicitly
synchronized at the API level.
v2: Opt out of tracking more aggressively (Ken): In addition to the
above, surface states, binding tables, instructions and most
dynamic states are now left untracked, which means a *lot* more BO
uses marked IRIS_DOMAIN_NONE which need to be reviewed extremely
carefully, since the cache tracker won't be able to provide any
coherency guarantees for them.
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/3875>
For temporary lookups, just allocate out of the NULL ralloc context,
so we don't have to edit the linked list of ralloc children to add it
and then immediately remove it again.
When uploading a new shader, allocate the keybox off the shader, so
if we delete the shader the keybox also goes away. Less manual cleanup.
Right now, all keys have two things in common: a program string ID and a
sampler_prog_key_data. I'd like to add another thing or two and need a
place to put it. This commit adds a new brw_base_prog_key struct which
contains those two common bits.
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Stop using brw_compiler to lower the final binding table indices for
surface access. This is done by simply not setting the
'prog_data->binding_table.*_start' fields. Then make the driver
perform this lowering.
This is a better place to perfom the binding table assignments, since
the driver has more information and will also later consume those
assignments to upload resources.
This also prepares us for two changes: use ibc without having to
implement binding table logic there; and remove unused entries from
the binding table.
Since the `block` field in brw_ubo_range now refers to the final
binding table index, we need to adjust it before using to index
shs->constbuf.
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
In st_nir_lower_uniforms_to_ubo() all UBO access in the shader have
its index incremented to open room for uniforms in constbuf0. So if
we use UBOs, we always need to include the extra binding entry in the
table.
To avoid doing this checks both when compiling the shader and when
assigning binding tables, store the num_cbufs in iris_compiled_shader.
Fixes a bunch of tests from Piglit and CTS that use UBOs but don't use
uniforms or system values. Note that some tests fitting this criteria
were passing because the UBOs were moved to be push
constants (avoiding the problem).
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
dead since my program cache API rework. we could still use it for one
function, but it's so trivial to pass the size, that it's probably not
worth the extra code
This exposes iris_upload_shader() without having to bind it, which will
be useful for precompiles. It also lets us examine the old programs and
flag dirty bits at a higher level, rather than cramming all that
knowledge into the cache layer.
we borrow the approach from anv rather than i965, as it works better
with pre-baked state that needs to contain scratch BO addresses
fixes a bunch of varying packing tests
3DSTATE_CONSTANT_* looks at prog_data->ubo_ranges. We were getting
saved by iris_set_constant_buffers() usually happening when changing
programs (as they usually change uniforms too), but with the clear
shader that doesn't use uniforms, we weren't getting one and were
leaving push constants enabled, screwing things up.
Also clean up a bit of a mess left by the hacks - we were missing
bindings in the VS/FS/CS case, among other issues...
some of them had typos, didn't say 'authors or copyright holders',
or other mistakes. This is now https://opensource.org/licenses/MIT
text, formatted consistently.
