This trivially corrects mesa 3ca1c221, which introduced a check that
crashes when a match is not found.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95005
Fixes: piglit.spec.glsl-1_50.compiler.interface-blocks-name-reused-globally-4.vert
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
At this point, it would require a logic error in nir_validate to not
have already populated this hashtable entry, but coverity doesn't
realize that:
CID 1265547 (#1 of 1): Dereference null return value (NULL_RETURNS)3.
dereference: Dereferencing a null pointer entry.
CID 1271039 (#1 of 1): Dereference null return value (NULL_RETURNS)3.
dereference: Dereferencing a null pointer entry.
Signed-off-by: Rob Clark <robclark@freedesktop.org>
Reviewed-by: Matt Turner <mattst88@gmail.com>
Not sure how coverity arrives at the conclusion that we can read comp[j]
unitialized (around line 204), other than not being aware that ncomp is
greater than 1 so it won't underflow in the 'if (tex->is_array)' case.
Signed-off-by: Rob Clark <robclark@freedesktop.org>
Reviewed-by: Matt Turner <mattst88@gmail.com>
Not 100% sure, but I think being an unsigned literal will help:
CID 1358505 (#1 of 1): Unintended sign extension
(SIGN_EXTENSION)sign_extension: Suspicious implicit sign extension:
load1->def.num_components with type unsigned char (8 bits, unsigned) is
promoted in load1->def.num_components * (load1->def.bit_size / 8) to
type int (32 bits, signed), then sign-extended to type unsigned long (64
bits, unsigned). If load1->def.num_components * (load1->def.bit_size /
8) is greater than 0x7FFFFFFF, the upper bits of the result will all be
1.
Signed-off-by: Rob Clark <robclark@freedesktop.org>
Reviewed-by: Matt Turner <mattst88@gmail.com>
This works around a bug in older version of UE4, where a shader
defines the same structure twice. Although we aren't sure this is correct
GLSL (it most likely isn't) there are enough UE4 based things out there
we should deal with this.
This drops the error to a warning if the struct names and contents match.
v1.1: do better C++ on record_compare declaration (Rob)
v2: restrict this to desktop GL only (Ian)
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95005
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
This is my attempt at fixing at least one of the UE4 bugs with GL4.3.
If we are doing intrastage matching and hit anonymous structs, then
we should do a record comparison instead of using the names.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95005
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Mark it as unreachable. Silences a compiler warning:
spirv/spirv_to_nir.c:1397:4: warning: enumeration value
'nir_texop_txf_ms_mcs' not handled in switch [-Wswitch]
switch (instr->op) {
^
Reviewed-by: Jason Ekstrand <jason.ekstrand@intel.com>
layout should only be null for structs, but it's checked everywhere else
and confuses Coverity (CID 1358495).
Reviewed-by: Timothy Arceri <timothy.arceri@collabora.com>
Operations like nir_op_bitfield_insert have four arguments, and Coverity
isn't privy to the fact that 4-argument operations aren't possible here,
so it thinks this can lead to memory corruption. Just increase the size
of the array to quell any fears.
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
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>
Log all the errors, and at the end dump the shader w/ error annotations
to make it easier to see where the problems are.
Signed-off-by: Rob Clark <robclark@freedesktop.org>
Reviewed-by: Eduardo Lima Mitev <elima@igalia.com>
Reviewed-by: Connor Abbott <cwabbott0@gmail.com>
Caller can pass a hashtable mapping NIR object (currently instr or var,
but I guess others could be added as needed) to annotation msg to print
inline with the shader dump. As the annotation msg is printed, it is
removed from the hashtable to give the caller a way to know about any
unassociated msgs.
This is used in the next patch, for nir_validate to try to associate
error msgs to nir_print dump.
Signed-off-by: Rob Clark <robclark@freedesktop.org>
Reviewed-by: Eduardo Lima Mitev <elima@igalia.com>
Reviewed-by: Connor Abbott <cwabbott0@gmail.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've added this to nir_gather_info(), but also to glsl_to_nir() as a
temporary measure, since the i965 GL driver today doesn't use
nir_gather_info() yet.
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
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>
Drop extra #include which is otherwise unneeded (and makes this header
difficult to include from outside of src/mesa).
Signed-off-by: Rob Clark <robclark@freedesktop.org>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
With algebraic-opt support for lowering div to shift, the driver would
like to be able to run this pass *after* the main opt-loop, and then
conditionally re-run the opt-loop if this pass actually lowered some-
thing.
Signed-off-by: Rob Clark <robclark@freedesktop.org>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Intel hardware does a form of multisample compression that involves an
auxilary surface called the MCS. When an MCS is in use, you have to first
sample from the MCS with a special opcode and then pass the result of that
operation into the next sample instrucion. Normally, we just do this
ourselves in the back-end, but we want to expose that functionality to NIR
so that we can use MCS values directly in NIR-based blorp.
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
This is similar to nir_channel except that it lets you grab more than one
channel by providing a mask.
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
There's no reason for having a macro *and* a python generator. We can
easily just do the whole thing in python. This has the advantage that we
are no longer definining ALU# macros which conflict with the ones in
brw_fs_builder.h.
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
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>
