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>
%ld and %lu aren't the right format specifiers for int64_t and uint64_t
on 32-bit (x86) systems. They're %zu on Linux and %Iu on Windows.
Use the standard C99 macros in hopes that they work everywhere.
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
lower_variable_index_to_cond_assign() did not handle system values.
gl_SampleMaskIn[] is a system value, and also an array. Accessing it
with a variable index would trigger an unreachable() assert.
Rather than adding a new EmitNoIndirectSystemValues flag, we simply
lower unconditionally. There is exactly one case where this occurs,
and for all current drivers, lowering produces optimal code. Even
for future drivers with 32x MSAA, it produces reasonable code.
Fixes Piglit's new samplemaskin-indirect test. Also fixes many ES31-CTS
tests when OES_sample_variables is enabled.
Cc: mesa-stable@lists.freedesktop.org
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Ilia Mirkin <imirkin@alum.mit.edu>
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
In the first pass of implementing exact handling, I made a mistake with
search-and-replace. In particular, we only reallly handled exact/inexact
on the root of the tree. Instead, we need to check every node in the tree
for an exact/inexact match. As an example of this, consider the following
GLSL code
precise float a = b + c;
if (a < 0) {
do_stuff();
}
In that case, only the add will be declared "exact" and an expression that
looks for "b + c < 0" will still match and replace it with "b < -c" which
may yield different results. The solution is to simply bail if any of the
values are exact when matching an inexact expression.
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
Also avoid double-adding the *sampler2DMS types when the array ext is
enabled.
Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
Reviewed-by: Dave Airlie <airlied@redhat.com>
I can't tell whether this actually matters, but we're creating function
signatures with this predicate, so it should probably match when SSBO's
are available.
Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
Reviewed-by: Dave Airlie <airlied@redhat.com>
Oddly a bunch of the features it adds are actually from ESSL 3.20. But
the spec is quite clear, oh well.
Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
Reviewed-by: Dave Airlie <airlied@redhat.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>
Since 8683d54d2b there is now a single instance of the buffer
block information that needs to be updated rather than one instance
for each stage.
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>
If the GL_ARB_shader_draw_parameters extension is enabled, we'll already
have a gl_BaseVertex variable. It will have var->how_declared set to
ir_var_declared_implicitly, and will appear in the program resource
list.
If not, we make one for internal use. We don't want it to be listed
in the program resource list, as the application won't be expecting
it. Marking it hidden will properly exclude it.
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Timothy Arceri <timothy.arceri@collabora.com>
Reviewed-by: Ilia Mirkin <imirkin@alum.mit.edu>
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>
Note: This patch appears to violate older OpenGL and OpenGLES specs.
The OpenGLES GLSL 3.1 and OpenGL GLSL 4.3 specifications both remove
the requirement for the output and input centroid qualifiers to match.
The deqp
dEQP-GLES3.functional.shaders.linkage.varying.rules.differing_interpolation_2
test wants the newer OpenGLES 3.1 specification behavior, even for
OpenGLES 3.0. This patch simply removes the checking in all cases.
The OpenGLES 3.0 conformance test suite doesn't appear to require the
older ("must match") spec behavior.
For reference, here are the relavent spec citations:
The OpenGL 4.2 spec says: "the last active shader stage output
variables and fragment shader input variables of the same name must
match in type and qualification (other than out matching to in)"
The OpenGL 4.3 spec says: "interpolation qualification (e.g., flat)
and auxiliary qualification (e.g. centroid) may differ."
The OpenGLES GLSL 3.00.4 specification says: "The output of the
vertex shader and the input of the fragment shader form an
interface. For this interface, vertex shader output variables and
fragment shader input variables of the same name must match in type
and qualification (other than precision and out matching to in)."
The OpenGLES GLSL 3.10 Specification says: "interpolation
qualification (e.g., flat) and auxiliary qualification (e.g.
centroid) may differ"
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92743
Bugzilla: https://cvs.khronos.org/bugzilla/show_bug.cgi?id=7819
Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ian Romanick <ian.d.romanick@intel.com>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
It used to be in nir_gather_info.c until I moved it out to nir.h so it
could be re-used with some linking code that never got merged. We'll move
it back out if and when we have real code to share it with.
Just noticed this in passing.. gl_shader_stage already has tess so this
comment no longer applies.
Signed-off-by: Rob Clark <robclark@freedesktop.org>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Needed because not all the built-in variables are marked as system
values, so they still have the mode ir_var_auto. Right now it fixes
raising the warning when gl_GlobalInvocationID and
gl_LocalInvocationIndex are used.
v2: use is_gl_identifier instead of filtering for some names (Ilia
Mirkin)
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Oddly, this did not affect the shader where I first noticed the pattern.
That particular shader doesn't get its if-statement converted to a bcsel
because there are two assignments in the else-statement. This led to me
submitting https://bugs.freedesktop.org/show_bug.cgi?id=94747.
shader-db results:
Sandy Bridge
total instructions in shared programs: 8467384 -> 8467069 (-0.00%)
instructions in affected programs: 36594 -> 36279 (-0.86%)
helped: 46
HURT: 0
total cycles in shared programs: 117573448 -> 117568518 (-0.00%)
cycles in affected programs: 339114 -> 334184 (-1.45%)
helped: 46
HURT: 0
Ivy Bridge / Haswell / Broadwell / Skylake:
total instructions in shared programs: 7774258 -> 7773999 (-0.00%)
instructions in affected programs: 30874 -> 30615 (-0.84%)
helped: 46
HURT: 0
total cycles in shared programs: 65739190 -> 65734530 (-0.01%)
cycles in affected programs: 180380 -> 175720 (-2.58%)
helped: 45
HURT: 1
No change on G45 or Ironlake.
I also tried these expressions, but none of them affected any shaders in
shader-db:
(('bcsel', a, 'a@bool', 'b@bool'), ('ior', a, b)),
(('bcsel', a, 'b@bool', False), ('iand', a, b)),
(('bcsel', a, 'b@bool', 'a@bool'), ('iand', a, b)),
Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
