It is currently a bitset on top of a uint64_t but there are already
more than 64 values. Change to use BITSET to cover all the
SYSTEM_VALUE_MAX bits.
Cc: mesa-stable
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Karol Herbst <kherbst@redhat.com>
Acked-by: Jesse Natalie <jenatali@microsoft.com>
Acked-by: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
Acked-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
Acked-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8585>
Previously, we had two field in shader_info: double_inputs_read and
double_inputs. Presumably, the one was for all double inputs that are
read and the other is all that exist. However, because nir_gather_info
regenerates these two values, there is a possibility, if a variable gets
deleted, that the value of double_inputs could change over time. This
is a problem because double_inputs is used to remap the input locations
to a two-slot-per-dvec3/4 scheme for i965. If that mapping were to
change between glsl_to_nir and back-end state setup, we would fall over
when trying to map the NIR outputs back onto the GL location space.
This commit changes the way slot re-mapping works. Instead of the
double_inputs field in shader_info, it adds a DualSlotInputs bitfield to
gl_program. By having it in gl_program, we more easily guarantee that
NIR passes won't touch it after it's been set. It also makes more sense
to put it in a GL data structure since it's really a mapping from GL
slots to back-end and/or NIR slots and not really a NIR shader thing.
Tested-by: Alejandro Piñeiro <apinheiro@igalia.com> (ARB_gl_spirv tests)
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
- remove mtypes.h from most header files
- add main/menums.h for often used definitions
- remove main/core.h
v2: fix radv build
Reviewed-by: Brian Paul <brianp@vmware.com>
First we move double_inputs_read into a vs struct in the union,
double_inputs_read is only used for vs inputs so this will
save space and also allows us to add a new double_inputs field.
We add the new field because c2acf97fcc changed the behaviour
of double_inputs_read, and while it's no longer used to track
actual reads in i965 we do still want to track this for gallium
drivers.
Reviewed-by: Marek Olšák <marek.olsak@amd.com>
I apparently broke mark_whole_variable in ir_set_program_inouts.
It was passing a type that wasn't var->type, so the wrapper didn't
work out. It's all broken, revert it and start over.
Fixes all kinds of things on other drivers.
Revert "glsl: Make is_fixed_function_array actually check for varyings."
This reverts commit 42699e1271.
Revert "glsl: Mark whole variable used for ClipDistance and TessLevel*."
This reverts commit 5c580e64cc.
Revert "glsl: Override the # of varying slots for ClipDistance and TessLevel*."
This reverts commit 8b5749f65a.
Revert "glsl: Create and use a new ir_variable::count_attribute_slots() wrapper."
This reverts commit 6aa5cb34d0.
We can't check VARYING_SLOT_* locations until we've determined that
the variable is actually a varying.
Fixes assert failures in drivers which actually use this path,
such as radeonsi and i915.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99314
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
There's no point in trying to mark partial array access for
gl_ClipDistance, gl_TessLevelOuter, or gl_TessLevelInner - they're
special built-in variables that control fixed function hardware,
and will likely be used in an all-or-nothing fashion.
Since these arrays only occupy 1-2 varying slots, we have to avoid
our normal processing which increments the slot value by the array
index.
(I wrote this code before i965 switched from ir_set_program_inouts
to nir_shader_gather_info. It's not used by anyone today, and I'm
not sure how valuable it is...the alternative to GLSL IR lowering
is NIR compact arrays, at which point you should use nir_gather_info.)
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Timothy Arceri <timothy.arceri@collabora.com>
This wraps glsl_type::count_attribute_slots(), but will soon contain a
couple of overrides for a couple of GLSL built-ins variables.
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Timothy Arceri <timothy.arceri@collabora.com>
And set system values read directly in shader_info.
st/mesa changes where:
Reviewed-by: Marek Olšák <marek.olsak@amd.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
And set outputs written directly in shader_info.
st/mesa changes where:
Reviewed-by: Marek Olšák <marek.olsak@amd.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
And set set inputs_read directly in shader_info.
To avoid regressions between changes this change is a squashed
version of the following patches.
st/mesa changes where:
Reviewed-by: Marek Olšák <marek.olsak@amd.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Note we access shader_info from the program struct rather than the
nir_shader pointer because shader cache won't create a nir_shader.
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
In the fragment shader OutputsWritten is a bitset of FRAG_RESULT_*
enumerants, which represent the location of each color output written
by the shader. The secondary and primary color outputs of a given
render target using dual-source blending have the same location, so
the 'idx' computation below will give the wrong bit as result if the
'var->data.index' term is non-zero -- E.g. if the shader writes the
primary and secondary colors of the FRAG_RESULT_COLOR output,
ir_set_program_inouts will think that the shader writes both
FRAG_RESULT_COLOR and FRAG_RESULT_SAMPLE_MASK, which is just bogus.
That would cause the brw_wm_prog_key::nr_color_regions computation
done in the i965 driver during fragment shader precompilation to be
wrong, which currently leads to unnecessary recompilation of shaders
that use dual-source blending, and triggers an assertion failure in
fs_visitor::emit_fb_writes() on my i965-fb-fetch branch.
Reviewed-by: Ilia Mirkin <imirkin@alum.mit.edu>
This is the set of shader outputs whose initial value is provided to
the shader by some external means when the shader is executed, rather
than computed by the shader itself.
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Since they cannot be written. This prevents adding fragment outputs
to the OutputsWritten set that are only read from via the
gl_LastFragData array but never written to.
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
This assertion is bogus. Varying structs, and arrays of structs, are
allowed by GLSL, and we can see them here. While we currently don't
have any partial-variable support for those, simply returning false
and marking the entire thing as used is certainly legitimate.
I believe this is often swept under the rug by varying packing,
but that's disabled in certain tessellation situations.
Hit by 20 dEQP-GLES31.functional.tessellation.user_defined_io.* tests.
Cc: mesa-stable@lists.freedesktop.org
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Timothy Arceri <timothy.arceri@collabora.com>
Likewise, rename the enum type to glsl_interp_mode.
Beyond the GLSL front-end, talking about "interpolation modes" seems
more natural than "interpolation qualifiers" - in the IR, we're removed
from how exactly the source language specifies how to interpolate an
input. Also, SPIR-V calls these "decorations" rather than "qualifiers".
Generated by:
$ find . -regextype egrep -regex '.*\.(c|cpp|h)' -type f -exec sed -i \
-e 's/INTERP_QUALIFIER_/INTERP_MODE_/g' \
-e 's/glsl_interp_qualifier/glsl_interp_mode/g' {} \;
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Acked-by: Dave Airlie <airlied@redhat.com>
In the future int64 support will have the same requirements.
Reviewed-by: Ilia Mirkin <imirkin@alum.mit.edu>
Signed-off-by: Dave Airlie <airlied@redhat.com>
This catches a problem previously undetected until deep in the backend.
Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
Reviewed-by: Timothy Arceri <timothy.arceri@collabora.com>
vertex_input_slots would be an appropriate name for an integer, but not
a bool.
Also remove a cond ? true : false from a count_attribute_slots() call
site, noticed during the rename.
Reviewed-by: Timothy Arceri <timothy.arceri@collabora.com>
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
