V2: fix error checking for arrays and components. V1 was
only taking into account all the array elements and all the
components of one of the varyings during the comparision
and treating the other as a single slot/component.
Reviewed-by: Anuj Phogat <anuj.phogat@gmail.com>
This prevents array overflow when the block is actually an array of UBOs or
SSBOs. On some hardware such as i965, such overflows can cause GPU hangs.
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
The last version of this broke clipping, and I had to spend
sometime getting this working properly.
I had to introduce a third pass to count the clip/cull totals,
all due to one messy corner case. We have a piglit test
tes-input-gl_ClipDistance.shader_test
that doesn't actually output the clip distances, it just passes
them like a varying from TCS->TES, the older lowering pass worked
but to lower clip/cull we need to know the total number of clip+culls
used to defined the new variable correctly, and to offset culls
properly.
This adds an extra pass that works out the sizes for clip/cull,
then lowers gl_ClipDistance then gl_CullDistance into the new
gl_ClipDistanceMESA.
The pass checks using the fixed array sizes code if they array
has been referenced, or is actually never used, and ignores
it in the latter case.
Reviewed-by: Ilia Mirkin <imirkin@alum.mit.edu>
Signed-off-by: Dave Airlie <airlied@redhat.com>
This fixes a bug that breaks cull distances. The problem
is the max array accessors can't tell the difference between
an never accessed unsized array and an accessed at location 0
unsized array. This leads to converting an undeclared unused
gl_ClipDistance inside or outside gl_PerVertex to a size 1
array. However we need to the number of active clip distances
to work out the starting point for the cull distances, and
this offset by one when it's not being used isn't possible
to distinguish from the case were only the first element is
accessed. I tried to use ->used for this, but that doesn't
work when gl_ClipDistance is part of an interface block.
So this changes things so that max_array_access is an int
and initialised to -1. This also allows unsized arrays to
proceed further than that could before, but we really shouldn't
mind as they will get eliminated if nothing uses them later.
For initialised uniforms we no longer change their array
size at runtime, if these are unused they will get eliminated
eventually.
v2: use ralloc_array (Ilia)
Reviewed-by: Ilia Mirkin <imirkin@alum.mit.edu>
Signed-off-by: Dave Airlie <airlied@redhat.com>
From the GL 4.5 core spec, section 11.1.1 (Vertex Attributes):
"A program with more than the value of MAX_VERTEX_ATTRIBS
active attribute variables may fail to link, unless
device-dependent optimizations are able to make the program
fit within available hardware resources. For the purposes
of this test, attribute variables of the type dvec3, dvec4,
dmat2x3, dmat2x4, dmat3, dmat3x4, dmat4x3, and dmat4 may
count as consuming twice as many attributes as equivalent
single-precision types. While these types use the same number
of generic attributes as their single-precision equivalents,
implementations are permitted to consume two single-precision
vectors of internal storage for each three- or four-component
double-precision vector."
This commits makes dvec3, dvec4, dmat2x3, dmat2x4, dmat3, dmat3x4,
dmat4x3 and dmat4 consume twice as many attributes as equivalent
single-precision types.
v3: count doubles as consuming two attributes (Dave Airlie)
v4: make reference to spec (Michael Schellenberger Costa)
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Dave Airlie <airlied@redhat.com>
Signed-off-by: Antia Puentes <apuentes@igalia.com>
Signed-off-by: Juan A. Suarez Romero <jasuarez@igalia.com>
The code didn't deal with explicit function indexes properly.
It also handed out the indexes at link time, when we really
need them in the lowering pass to create the correct if ladder.
So this patch moves assigning the non-explicit indexes earlier,
fixes the lowering pass and the lookups to get the correct values.
This fixes a few of:
GL45-CTS.explicit_uniform_location.subroutine-index-*
Signed-off-by: Dave Airlie <airlied@redhat.com>
The code was implementing the ACTIVE_SUBROUTINE_UNIFORMS
incorrectly, using the number of types not the number of
uniforms. This is different than the locations as the
locations may be sparsly allocated.
This fixes:
GL43-CTS.shader_subroutine.four_subroutines_with_two_uniforms
Reviewed-by: Chris Forbes <chrisforbes@google.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
This fixes a crash in:
GL45-CTS.explicit_uniform_location.subroutine-loc-negative-link-max-num-of-locations
Reviewed-by: Chris Forbes <chrisforbes@google.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
This fixes:
GL45-CTS.explicit_uniform_location.subroutine-index-negative-link-max-num-of-indices
Reviewed-by: Chris Forbes <chrisforbes@google.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
If a subroutine uniform is declared with no functions backing it,
that isn't legal, so we should fail to link.
Fixes:
GL43-CTS.shader_subroutine.subroutine_uniform_wo_matching_subroutines
Reviewed-by: Chris Forbes <chrisforbes@google.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Previously an SSO pipeline containing only a tessellation control shader
and a tessellation evaluation shader would not get locations assigned
for the TCS inputs. This would lead to assertion failures in some
piglit tests, such as arb_program_interface_query-resource-query.
That piglit test still fails on some tessellation related subtests.
Specifically, these subtests fail:
'GL_PROGRAM_INPUT(tcs) active resources' expected 2 but got 3
'GL_PROGRAM_INPUT(tcs) max length name' expected 12 but got 16
'GL_PROGRAM_INPUT(tcs,tes) active resources' expected 2 but got 3
'GL_PROGRAM_INPUT(tcs,tes) max length name' expected 12 but got 16
'GL_PROGRAM_OUTPUT(tcs) active resources' expected 15 but got 3
'GL_PROGRAM_OUTPUT(tcs) max length name' expected 23 but got 12
Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
Reviewed-by: Timothy Arceri <timothy.arceri@collabora.com>
Cc: mesa-stable@lists.freedesktop.org
Commit 11096ec introduced a regression in some piglit tests (e.g.,
arb_program_interface_query-resource-query). I did not notice this
regression because other (unrelated) problems caused failed assertions
in those same tests on my system... so they crashed before getting to
the new failure.
v2: Use is_gl_identifier. Suggested by Tim.
Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
Reviewed-by: Timothy Arceri <timothy.arceri@collabora.com>
Cc: mesa-stable@lists.freedesktop.org
The parameter appears to have been unused since the function was added
in commit 12ba6cfb. Remove it.
glsl/linker.cpp:2886:60: warning: unused parameter ‘prog’ [-Wunused-parameter]
match_explicit_outputs_to_inputs(struct gl_shader_program *prog,
^
Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
Reviewed-by: Timothy Arceri <timothy.arceri@collabora.com>
These varying have a separate location domain from per-vertex varyings
and need to be handled separately.
Reviewed-by: Dave Airlie <airlied@redhat.com>
On my oes_shader_io_blocks branch, this fixes 71
dEQP-GLES31.functional.program_interface_query.* tests.
Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Cc: mesa-stable@lists.freedesktop.org
v2: make too large array a compile error
v3: squash mesa/prog patch to avoid static compiler errors in bisect
Signed-off-by: Tobias Klausmann <tobias.johannes.klausmann@mni.thm.de>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Reviewed-by: Kristian Høgsberg <krh@bitplanet.net>
This will come in handy when we want to lower gl_CullDistance into
gl_CullDistanceMESA.
[airlied: drop separate APIs for clip/cull - just use single API
to call both passes.]
v3: reexamine my sanity, this was pretty broken, the new code
creates one copy of gl_ClipDistanceMESA, as the clip distance
varying and lowers everything into that in two passes, one for clips
one for culls.
v4: rework using the passes in clip/cull sizes, instead of the
array sizes.
Signed-off-by: Tobias Klausmann <tobias.johannes.klausmann@mni.thm.de>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Reviewed-by: Kristian Høgsberg <krh@bitplanet.net>
airlied:
v2: rename LowerClipDistance to LowerCombinedClipCullDistnace.
I don't think we want any other behaviour with any current hw.
Signed-off-by: Tobias Klausmann <tobias.johannes.klausmann@mni.thm.de>
Reviewed-by: Edward O'Callaghan <eocallaghan@alterapraxis.com>
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
Reviewed-by: Kristian Høgsberg <krh@bitplanet.net>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Currently, when cross validating global variables, all global variables
seen in the shaders that are part of a program are saved in a table.
When checking a variable this already exist in the table, we check both
are initialized to the same value. If the already saved variable does
not have an initializer, we copy it from the new variable.
Unfortunately this is wrong, as we are modifying something it is
constant. Also, if this modified variable is used in
another program, it will keep the initializer, when it should have none.
Instead of copying the initializer, this commit replaces the old
variable with the new one. So if we see again the same variable with an
initializer, we can compare if both are the same or not.
v2: convert tabs in whitespaces (Kenenth Graunke)
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
From Section 7.3.1.1 (Naming Active Resources) of the OpenGL 4.5 spec:
"For the property LOCATION_COMPONENT, a single integer indicating the first
component of the location assigned to an active input or output variable is
written to params. For input and output variables with a component specified
by a layout qualifier, the specified component is written. For all other
input and output variables, the value zero is written."
Reviewed-by: Anuj Phogat <anuj.phogat@gmail.com>
This is needed so we don't optimise away the varying when more than
one shares the same location.
Reviewed-by: Anuj Phogat <anuj.phogat@gmail.com>
Reviewed-by: Edward O'Callaghan <eocallaghan@alterapraxis.com>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
lower_vector_derefs can produce new vector_extract operations.
Neither i965 nor st_glsl_to_tgsi can handle them, so we'd best
convert them to swizzles.
Together with the previous patch, this fixes assertion failures in
GLideN64, as well as a new Piglit test which reproduces the issue:
spec/glsl-1.10/compiler/vector-dereference-in-dereference.frag
Cc: mesa-stable@lists.freedesktop.org
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95164
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Ian Romanick <ian.d.romanick@intel.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>
ARB_program_interface_query requires that we add struct fields
recursively down to basic types.
Fixes 52 struct test cases in dEQP-GLES31.functional.program_interface_query.*
Signed-off-by: Kristian Høgsberg Kristensen <kristian.h.kristensen@intel.com>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
No functional change here, but this now lets us recurse throught structs
in add_shader_variable().
Signed-off-by: Kristian Høgsberg Kristensen <kristian.h.kristensen@intel.com>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
This lets us pass in the absolution location of a variable instead of
computing it in add_shader_variable() based on variable location and
bias. This is in preparation for recursing into struct variables.
Signed-off-by: Kristian Høgsberg Kristensen <kristian.h.kristensen@intel.com>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
This consolidates the combination of create_shader_variable() and
add_program_resource() into a new helper function. No functional
difference, but we'll expand add_shader_variable() in the next few
commits.
Signed-off-by: Kristian Høgsberg Kristensen <kristian.h.kristensen@intel.com>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
v2: change check_explicit_uniform_locations() to return an
unsigned 0 (Timothy Arceri)
We were storing the int result of check_explicit_uniform_locations()
in num_explicit_uniform_locs as an unsigned int which caused it to
be 4294967295 when a -1 was returned.
This in turn would cause the following error during linking:
error: count of uniform locations > MAX_UNIFORM_LOCATIONS(4294967295 > 98304)
Results from running piglit tests/all with this patch
and when ARB_explicit_uniform_location disabled:
changes: 178
fixes: 176
regressions: 2
The two regressions are for the following tests:
glean@glsl1-matrix column check (1)
glean@glsl1-matrix column check (2)
which regress from FAIL to CRASH.
The regressions are acceptable because the tests are currently failing due to
the aforementioned linker error.
Signed-off-by: Lars Hamre <chemecse@gmail.com>
Reviewed-by: Timothy Arceri <timothy.arceri@collabora.com>
With this change we create the UBO and SSBO arrays separately from the
beginning rather than putting them into a combined array and splitting
it apart later.
A bug is with UBO and SSBO stage reference querying is also fixed as
we now use the block index to lookup the references in the separate arrays
not the combined buffer block array.
Reviewed-by: Samuel Iglesias Gonsálvez <siglesias@igalia.com>
This allows us to simplify the code and drop InterfaceBlockStageIndex
which is a per stage array of integers the size of all blocks in the
program combined including duplicates across stages. Adding a stage
ref per block will use less memory.
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
This changes the code to use the buffer counts stored for each stage
rather than counting from scratch. It also moves the checks outside
of the for loop which means we now just get a single link error
message if we go over the max rather than X error messages where X
is the number we have exceeded the max by.
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
This will allow us to use them when checking resources in a
following patch and clean up a bunch of code.
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
With SSO, the GL_PROGRAM_INPUT and GL_PROGRAM_OUTPUT interfaces refer to
the first and last shader stage linked into a program. This may not be
the vertex and fragment shader stages.
So, subtracting VERT_ATTRIB_GENERIC0 and FRAG_RESULT_DATA0 is bogus.
We need to subtract VERT_ATTRIB_GENERIC0 for VS inputs,
FRAG_RESULT_DATA0 for FS outputs, and VARYING_SLOT_VAR0 for other cases.
Note that built-in variables get a location of -1.
Fixes 4 dEQP-GLES31.functional.program_interface_query tests:
- program_input.location.separable_fragment.var_explicit_location
- program_input.location.separable_fragment.var_array_explicit_location
- program_output.location.separable_vertex.var_array_explicit_location
- program_output.location.separable_vertex.var_array_explicit_location
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Timothy Arceri <timothy.arceri@collabora.com>
We were recording locations for all variables, even ones without an
explicit location set. Implement the rules from the spec, and record
-1 in the resource list accordngly. Make program_resource_location
stop doing math on negative values. Remove hacks that are no longer
necessary now that we've stopped doing that.
Fixes 4 dEQP-GLES31.functional.program_interface_query tests:
- program_input.location.separable_fragment.var
- program_input.location.separable_fragment.var_array
- program_output.location.separable_vertex.var_array
- program_output.location.separable_vertex.var_array
v2: Delete more code
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Timothy Arceri <timothy.arceri@collabora.com>
A program will either have gl_VertexID or gl_VertexIDMESA (the lowered
zero-based version), not both. Just spoof it in the resource list so
the hacks are done in a single place.
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Timothy Arceri <timothy.arceri@collabora.com>
System values are just built-in input variables that we've opted to
special-case out of convenience. We need to consider all inputs,
regardless of how we've classified them.
Unfortunately, there's one exception: we shouldn't add gl_BaseVertex
unless ARB_shader_draw_parameters is enabled, because it doesn't
actually exist in the language, and shouldn't be counted in the
GL_ACTIVE_RESOURCES query.
Fixes dEQP-GLES31.functional.program_interface_query.program_input.
resource_list.compute.empty, which expects gl_NumWorkGroups to appear
in the resource list.
v2: Delete more code
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Timothy Arceri <timothy.arceri@collabora.com>
This makes no sense. If the stage being considered is the vertex
shader, then we'll add inputs and system values appropriately.
If we're not considering the vertex shader, then we absolutely should
not do anything with it.
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Timothy Arceri <timothy.arceri@collabora.com>
add_interface_variables() is supposed to add variables for the inputs of
the first shader stage linked into a program, and the outputs of the
last shader stage linked into a program.
From the ARB_program_interface_query specification:
"* PROGRAM_INPUT corresponds to the set of active input variables used by
the first shader stage of <program>. If <program> includes multiple
shader stages, input variables from any shader stage other than the
first will not be enumerated.
* PROGRAM_OUTPUT corresponds to the set of active output variables
(section 2.14.11) used by the last shader stage of <program>. If
<program> includes multiple shader stages, output variables from any
shader stage other than the last will not be enumerated."
Previously, we used build_stageref here, which walks over all linked
shaders in the program. This meant that internal varyings would be
visible. We don't actually need any of build_stageref's code: we
already explicitly skip packed varyings, handle modes, and the name
comparisons just do a fuzzy string comparison of name with itself.
Fixes two tests: dEQP-GLES31.functional.program_interface_query.
program_{input,output}.referenced_by.referenced_by_vertex_fragment.
These tests have a VS and FS linked together into a single program.
Both stages have an input called "shaderInput". But the FS input
should not be visible because it isn't the first stage.
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Timothy Arceri <timothy.arceri@collabora.com>
This is a bitfield of which stages refer to a variable. It is not used
to mask off bits. In fact, it's used to contribute additional bits.
Rename it and tidy a bit of the logic.
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Timothy Arceri <timothy.arceri@collabora.com>
add_interface_variables is supposed to add variables from either the
first or last stage of a linked shader. But it has no way of knowing
the stage it's being asked to process, which makes it impossible to
produce correct stagerefs.
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Timothy Arceri <timothy.arceri@collabora.com>
We occasionally generate variables internally that we want to exclude
from the program resource list, as applications won't be expecting them
to be present.
The next patch will make use of this.
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Timothy Arceri <timothy.arceri@collabora.com>
Reviewed-by: Ilia Mirkin <imirkin@alum.mit.edu>
