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
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>
The use of the parameter was removed in d6b92028.
glsl/link_varyings.cpp:1390:39: warning: unused parameter ‘separate_shader’ [-Wunused-parameter]
bool separate_shader)
^
Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
Reviewed-by: Timothy Arceri <timothy.arceri@collabora.com>
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>
These varyings 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
opt_constant_folding is supposed to fold trees of constants into a
single constant. Surprisingly, it was also propagating constant values
from variables into expression trees - even when the result couldn't be
folded together. This is opt_constant_propagation's job.
The ir_dereference_variable::constant_expression_value() method returns
a clone of var->constant_value. So we would replace the dereference
with a constant, propagating it into the tree.
Skip over ir_dereference_variable to avoid this surprising behavior.
However, add code to explicitly continue doing it in the constant
propagation pass, as it's useful to do so.
shader-db statistics on Broadwell:
total instructions in shared programs: 8905349 -> 8905126 (-0.00%)
instructions in affected programs: 30100 -> 29877 (-0.74%)
helped: 93
HURT: 20
total cycles in shared programs: 71017030 -> 71015944 (-0.00%)
cycles in affected programs: 132456 -> 131370 (-0.82%)
helped: 54
HURT: 45
The only hurt programs are by a single instruction, while the helped
ones are helped by 1-4 instructions.
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
If an ir_dereference_array has non-constant components, there's no
point in trying to evaluate its value (which involves walking down
the tree and possibly allocating memory for portions of the subtree
which are constant).
This also removes convoluted tree walking in opt_constant_folding(),
which tries to fold constants while walking up the tree. No need to
walk down, then up, then down again.
We did this for swizzles and expressions already, but I was lazy
back in the day and didn't do this for ir_dereference_array.
No change in shader-db.
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
We could probably clean this up more (maybe make it a method), but at
least there's only one copy of this code now, and that's a start.
No change in shader-db.
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
It looks like this was missed when converting opt_constant_folding()
from a hierarchical visitor to an rvalue visitor in 6606fde3.
ir_rvalue_visitor already processes values on the way back up the tree,
so we will have already visited every child node. There's no point in
doing it again.
No change in shader-db.
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
The pass ultimately skips over any entries with assignment_count != 1,
so there's no need to do further work once we've determined that there
are multiple assignments.
The constant value could be a large array (i.e. uvec4[327]), at which
point skipping the constant_expression_value() call (and the clone()
call within) can save us piles of memory.
No change in shader-db.
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
I don't know what the intention was here, but this function returns
void. We can't assert anything about its return value.
Fixes "make check" failures.
v2: Also fix prototype for the function (caught by Jordan).
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
Many were already marked as fs_only, but not all. This fixes the
remaining ir_txb entries.
Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
interpolateAt* can only take input variables or an element of an input
variable array. No structs.
Further, GLSL 4.40 relaxes the requirement to allow swizzles, so enable
that as well.
This fixes the following dEQP tests:
dEQP-GLES31.functional.shaders.multisample_interpolation.interpolate_at_sample.negative.interpolate_struct_member
dEQP-GLES31.functional.shaders.multisample_interpolation.interpolate_at_centroid.negative.interpolate_struct_member
dEQP-GLES31.functional.shaders.multisample_interpolation.interpolate_at_offset.negative.interpolate_struct_member
Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
Reviewed-by: Chris Forbes <chrisforbes@google.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
In the case of a constant, it might have been propagated through and
variable_referenced() returns NULL. Error out in that case.
Fixes 3 dEQP tests:
dEQP-GLES31.functional.shaders.multisample_interpolation.interpolate_at_sample.negative.interpolate_constant
dEQP-GLES31.functional.shaders.multisample_interpolation.interpolate_at_centroid.negative.interpolate_constant
dEQP-GLES31.functional.shaders.multisample_interpolation.interpolate_at_offset.negative.interpolate_constant
Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
Reviewed-by: Eduardo Lima Mitev <elima@igalia.com>
Reviewed-by: Chris Forbes <chrisforbes@google.com>
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>
This just renames the file in anticipation of adding cull lowering,
and renames the internals.
Signed-off-by: Tobias Klausmann <tobias.johannes.klausmann@mni.thm.de>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Reviewed-by: Edward O'Callaghan <eocallaghan@alterapraxis.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>
Class "ir_constant" had a bunch of constructors where the pointer member
"array_elements" had not been initialized. This could have lead to unsafe
code if something had tried to write anything to it. This patch fixes
this issue by initializing the pointer to NULL in all the constructors.
This issue was discovered by Coverity.
CID: 401603, 401604, 401605, 401610
Signed-off-by: Jakob Sinclair <sinclair.jakob@openmailbox.org>
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
We'll need this for a nir pass to lower builtin-uniform access.
Signed-off-by: Rob Clark <robclark@freedesktop.org>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
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>
This fixes two of the cases in
GL43-CTS.shader_subroutine.subroutines_not_allowed_as_variables_constructors_and_argument_or_return_types
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
resource just appears in GLSL 4.20 without any fanfare.
Fixes GL43-CTX.CommonBugs.CommonBug_ReservedNames
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
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>
I don't think this will do much as it's a compiler error
to use component without location which is already in the
table but its good to be consistent.
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
Reviewed-by: Samuel Iglesias Gonsálvez <siglesias@igalia.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>
This change checks for component overlap, including handling overlap of
locations and components by doubles. Previously there was no validation
for assigning explicit locations to a location used by the second half
of a double.
V3: simplify handling of doubles and fix double component aliasing
detection
V2: fix component matching for matricies
Reviewed-by: Anuj Phogat <anuj.phogat@gmail.com>
Reviewed-by: Edward O'Callaghan <eocallaghan@alterapraxis.com>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
We make use of the existing IR field location_frac used for tracking
component locations.
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>
The old visitor missed some cases. For example, it wouldn't handle
an ir_dereference_array with a vector_extract as the index.
Rather than trying to add the missing cases, just rewrite it as an
ir_rvalue_visitor. This makes it easy to replace any expression,
and is much less code.
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>
This generalizes the validation also to be done for variables inside
interface blocks, which, for some cases, was missing.
For a discussion about the additional validation cases included see
https://lists.freedesktop.org/archives/mesa-dev/2016-March/109117.html
and Khronos bug #15671.
Reviewed-by: Samuel Iglesias Gonsálvez <siglesias@igalia.com>
Signed-off-by: Andres Gomez <agomez@igalia.com>
Return statements in conditional blocks were not having their
output varyings lowered correctly.
This patch fixes the following piglit tests:
/spec/glsl-1.10/execution/vs-float-main-return
/spec/glsl-1.10/execution/vs-vec2-main-return
/spec/glsl-1.10/execution/vs-vec3-main-return
Signed-off-by: Lars Hamre <chemecse@gmail.com>
Reviewed-by: Timothy Arceri <timothy.arceri@collabora.com>
Trivial change. Removing unnecessary semi-colons from the code.
I don't have push access so someone reviewing this can push it.
Signed-off-by: Jakob Sinclair <sinclair.jakob@openmailbox.org>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Reviewed-by: Edward O'Callaghan <eocallaghan@alterapraxis.com>
Reviewed-by: Chad Versace <chad.versace@intel.com>
