This invalid memory access is a consequence of wrong assumptions,
for instance:
"prog->sh.data is NULL if it's ARB_fragment_program"
This issue is triggered with piglit/fp-formats -auto -fbo:
==9747==ERROR: AddressSanitizer: heap-use-after-free on address 0x007f7c812d90 at pc 0x007f833c09f8 bp 0x007fd7eca750 sp 0x007fd7eca768
READ of size 4 at 0x007f7c812d90 thread T0
#0 0x7f833c09f4 in st_get_sampler_views ../src/mesa/state_tracker/st_atom_texture.c:109
#1 0x7f833c0b48 in update_textures ../src/mesa/state_tracker/st_atom_texture.c:266
#2 0x7f82b2d120 in st_validate_state ../src/mesa/state_tracker/st_util.h:128
#3 0x7f82b2d120 in prepare_draw ../src/mesa/state_tracker/st_draw.c:88
#4 0x7f82b2de64 in st_draw_gallium ../src/mesa/state_tracker/st_draw.c:141
#5 0x7f83105940 in _mesa_draw_arrays ../src/mesa/main/draw.c:1202
#6 0x7f8d5fa5cc in piglit_draw_rect_from_arrays piglit/tests/util/piglit-util-gl.c:711
#7 0x7f8d5fac34 in piglit_draw_rect_custom piglit/tests/util/piglit-util-gl.c:833
#8 0x4019e0 in piglit_display piglit/tests/shaders/fp-formats.c:67
#9 0x7f8d643fc4 in run_test piglit/tests/util/piglit-framework-gl/piglit_fbo_framework.c:52
#10 0x401624 in main piglit/tests/shaders/fp-formats.c:39
Signed-off-by: Patrick Lerda <patrick9876@free.fr>
Reviewed-by: Marek Olšák <marek.olsak@amd.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/21175>
Lowered fp64 ops can blow up the loop bodies while still being
suitable for unrolling.
Allow for using different parameters to unroll loops with
soft fp64.
Reviewed-by: Mike Blumenkrantz <michael.blumenkrantz@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/18863>
Calling this directly in the linker code allows us to place it between
the varying linker and uniform linker calls which allows for better
optimisation/removal of uniforms.
Also in a later patch it allows us to insert a new nir based
lower_const_arrays_to_uniforms() call after the gl_nir_link_opts()
call. This is important because it allows the linking opts to
move constant arrays to later stages if possible before
lower_const_arrays_to_uniforms() turns them into uniforms.
Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/6541
Acked-by: Emma Anholt <emma@anholt.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/16770>
Some backends lower constant arrays to uniforms in GLSL IR. These
create so called hidden uniforms. Since we know these are added
per stage it is safe to remove them if we detect they are dead.
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
Acked-by: Emma Anholt <emma@anholt.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/16770>
This will allow us to disable the GLSL IR loop unroller in a
following patch and rely on the NIR loop unroller instead.
This allows the piglit test spec@!opengl 2.0@max-samplers border
to pass on the v3d rpi4 driver.
Reviewed-by: Emma Anholt <emma@anholt.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/16543>
These will be used by a new nir based glsl varying linker that will
add varyings directly to the list before the are packed and we lose
the information.
Reviewed-by: Marek Olšák <marek.olsak@amd.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/15731>
This will allow us to use this in future NIR linker work. It also makes
more sense to move it here as the classic drivers are gone, tgsi is
going away and we are merging more of the st into the gl common code.
Reviewed-by: Emma Anholt <emma@anholt.net>
Reviewed-by: Dave Airlie <airlied@redhat.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/14785>
This saves a lot of pointless gl.h includes across the board,
it moves the one place that needs GLenum into a separate file
only used in those passes that require it.
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/14605>
This just adds the structure with a name and its update function.
strlen and others will be added in the following commits. The idea is to
parse and analyze the name in advance to make glGetUniformLocation faster.
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/13507>
Now that nir_foreach_variable_with_modes can handle multiple modes at
one time, we can simplify things a bit and only walk the list once.
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/5966>
This is now possible as we do uniform linking via a nir based linker.
Shader-db results for IRIS (SKL):
total instructions in shared programs: 14947192 -> 14946397 (<.01%)
instructions in affected programs: 39498 -> 38703 (-2.01%)
helped: 230
HURT: 18
total cycles in shared programs: 324868402 -> 324847058 (<.01%)
cycles in affected programs: 706701 -> 685357 (-3.02%)
helped: 599
HURT: 449
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Eric Anholt <eric@anholt.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4797>
This helper reflects the rules we follow in the GLSL IR linker when
deciding if we can remove a dead uniform. This check is required to
avoid regressions when turning on NIR dead uniform clean up in the
following patch.
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Eric Anholt <eric@anholt.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4797>
Several files had a initial comment about the purpose of such files,
including a reference that the NIR linker was implemented with just
ARB_gl_spirv in mind.
Since the nice job Timothy is doing to use the NIR linker on GLSL,
that is not true anymore, so let's remove that reference and also
tweak some other comments.
Acked-by: Timothy Arceri <tarceri@itsqueeze.com>
Tested-by: Marge Bot <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4081>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4081>
This is adapted from the GLSL IR code but doesn't need to
iterate over the IR. I believe this also fixes a potential bug in
the GLSL IR code which potentially counts the same output twice.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
This will allow us to do some linking in NIR that was previously
done by the GLSL IR linker. To start with this just has calls for
linking atomics.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
A NIR based glsl linking function will be too different to the
spirv version to bother attempting any sharing. So lets change
the name to be explicit.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Here we use the NIR based builder to add everything to the resource
list execpt for SSO packed varyings. Since the details of those
varyings get lost during packing we leave the special handing to
the GLSL IR pass for now. In order to do this we add some bools
to the build resource list functions.
Using the NIR based resource list builder gets us a step closer to
using a native NIR based linker. It should also be faster than the
GLSL IR builder, one because the NIR optimisations should mean we
add less entries due to better optimisations, and two because nir
gives us better lists to work with and we don't need to walk the
entire IR to find the resources.
Ack-by: Alejandro Piñeiro <apinheiro@igalia.com>
This adds support for adding names of varying to the resource list
which is required for us to use this function with the glsl linker.
Support for names is optional for spirv which is why it had not been
added yet.
This is mostly a copy of the GLSL IR code adapted to nir.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Iterate the system values list when adding varyings to the program
resource list in the NIR linker. This is needed to avoid CTS
regressions when using the NIR to build the GLSL resource list in
an upcoming series. Presumably it also fixes a bug with the current
ARB_gl_spirv support.
Fixes: ffdb44d3a0 ("nir/linker: Add inputs/outputs to the program resource list")
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Perform all the NIR linking steps in order. Change iris and i965 to
use it. Suggested by Alejandro.
v2: Add gl_nir_linker_options struct.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com> [v1]
v2: added TODO comment hinting possible future refactoring of
nir_build_program_resource_list and build_program_resource_list,
to avoid code duplication (Alejandro, to explicitly reflect a
valid concern from Timothy during the review).
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
v2: "nir/linker: Use the stageref when adding UBO/SSBO resources"
squashed on this one (Timothy)
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
Replace calls to create hash tables and sets that use
_mesa_hash_pointer/_mesa_key_pointer_equal with the helpers
_mesa_pointer_hash_table_create() and _mesa_pointer_set_create().
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Acked-by: Eric Engestrom <eric@engestrom.ch>
So they are not exposed through the introspection API.
It is worth to note that the number of hidden uniforms of GLSL linking
vs SPIR-V linking would be somewhat different due the differen order
of the nir lowerings/optimizations.
For example: gl_FbWposYTransform. This is introduced as part of
nir_lower_wpos_ytransform. On GLSL that is executed after the IR-based
linking. So that means that on GLSL the UniformStorage will not
include this uniform. With the SPIR-V linking, that uniform is already
present, but marked as hidden. So it will be included on the
UniformStorage, but as hidden.
One alternative would create a special how_declared for that case, but
seemed an overkill. Using hidden should be ok as far as it is used
properly.
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
This function is equivalent to the linker.cpp
build_program_resource_list() but will extract the resources from NIR
shaders instead.
For now, only uniforms and program inputs are implemented.
v2: move from compiler/nir to compiler/glsl (Timothy Arceri)
v3: remove support for inputs, that is still WIP (spotted by Timothy
Arceri)
Signed-off-by: Eduardo Lima <elima@igalia.com>
Signed-off-by: Alejandro Piñeiro <apinheiro@igalia.com>
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>