Right now nir_instrs_equal() is tied pretty tightly to CSE, but we're
going to introduce the idea of an instruction set and tie it to that
instead. In anticipation of that, move this into its own file where
we'll add the rest of the instruction set implementation later.
v2: Rebase on texture support.
Reviewed-by: Jason Ekstrand <jason.ekstrand@intel.com>
Signed-off-by: Connor Abbott <cwabbott0@gmail.com>
Adds nir_src_is_dynamically_uniform which returns true if the source
is known to be dynamically uniform. This will be used in a later patch
to add a workaround for cases that only work with dynamically uniform
sources. Note that the function is not definitive, it can return false
negatives (but not false positives). Currently it only detects
constants and uniform accesses. It could easily be extended to include
more cases.
Reviewed-by: Matt Turner <mattst88@gmail.com>
Previously the name of the nir shader was being freed prematurely during
nir_sweep. Since 756613ed35 the name was later being used to generate
filenames for the optimiser debug output and these would end up with
garbage from the dangling pointer.
Co-authored-by: Neil Roberts <neil@linux.intel.com>
Reviewed-by: Matt Turner <mattst88@gmail.com>
Varyings can be considered inputs or outputs of a program only when
SSO is in use. With multi-stage programs, inputs contain only inputs
for first stage and outputs contains outputs of the final shader stage.
I've tested that fix works for Assault Android Cactus (demo version)
and does not cause Piglit or CTS regressions in glGetProgramiv tests.
Following ES 3.1 CTS separate shader tests that do query properties
of varyings in SSO shader programs pass:
ES31-CTS.program_interface_query.separate-programs-vertex
ES31-CTS.program_interface_query.separate-programs-fragment
Signed-off-by: Tapani Pälli <tapani.palli@intel.com>
Tested-by: Dieter Nützel <Dieter@nuetzel-hh.de>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92122
With NIR, it actually hurts things.
total instructions in shared programs: 6529329 -> 6528888 (-0.01%)
instructions in affected programs: 14833 -> 14392 (-2.97%)
helped: 299
HURT: 1
In all affected programs I inspected (including the single hurt one) the
pass CSE'd some multiplies and caused some reassociation (e.g., caused
(A * B) * C to be A * (B * C)) when the original intermediate result was
reused elsewhere.
Acked-by: Kenneth Graunke <kenneth@whitecape.org>
Fixes following Piglit test:
global-scope-binding-qualifier.frag
Signed-off-by: Tapani Pälli <tapani.palli@intel.com>
Reviewed-by: Samuel Iglesias Gonsálvez <siglesias@igalia.com>
The uniform will only be of a single type so store the data for
opaque types in a single array.
Cc: Francisco Jerez <currojerez@riseup.net>
Cc: Ilia Mirkin <imirkin@alum.mit.edu>
Geometry and tessellation shaders process multiple vertices; their
inputs are arrays indexed by the vertex number. While GLSL makes
this look like a normal array, it can be very different behind the
scenes.
On Intel hardware, all inputs for a particular vertex are stored
together - as if they were grouped into a single struct. This means
that consecutive elements of these top-level arrays are not contiguous.
In fact, they may sometimes be in completely disjoint memory segments.
NIR's existing load_input intrinsics are awkward for this case, as they
distill everything down to a single offset. We'd much rather keep the
vertex ID separate, but build up an offset as normal beyond that.
This patch introduces new nir_intrinsic_load_per_vertex_input
intrinsics to handle this case. They work like ordinary load_input
intrinsics, but have an extra source (src[0]) which represents the
outermost array index.
v2: Rebase on earlier refactors.
v3: Use ssa defs instead of nir_srcs, rebase on earlier refactors.
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Jason Ekstrand <jason.ekstrand@intel.com>
get_io_offset() already walks the dereference chain and discovers
whether or not we have an indirect; we can just return that rather than
computing it a second time via deref_has_indirect(). This means moving
the call a bit earlier.
By returning a nir_ssa_def *, we can pass back both an existence flag
(via NULL checking the pointer) and the value in one parameter. It
also simplifies the code somewhat. nir_lower_samplers works in a
similar fashion.
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Jason Ekstrand <jason.ekstrand@intel.com>
This is a common enough operation that it's nice to not have to think about
the arguments to foreach_list_typed every time.
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
That way, if we do the usual thing of multiplying vector_elements by
matrix_columns we get the actual number of components in the type as per
component_slots().
While we're at it, we also switch to using the actual C++ field
initializers for vector_elements and matrix_columns.
Reviewed-by: Dave Airlie <airlied@redhat.com>
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Commit 0a1adaf11d (nir: Report progress
from nir_lower_system_values().) introduced a bug caught by Valgrind:
==823== Conditional jump or move depends on uninitialised value(s)
==823== at 0xB09020C: convert_block (nir_lower_system_values.c:68)
==823== by 0xB079FB8: foreach_cf_node (nir.c:1310)
==823== by 0xB07A0AF: nir_foreach_block (nir.c:1336)
==823== by 0xB09026B: convert_impl (nir_lower_system_values.c:79)
...
==823== Uninitialised value was created by a stack allocation
==823== at 0xB090249: convert_impl (nir_lower_system_values.c:76)
which is trivially fixed by initializing progress.
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Fixes following Piglit test:
member-invalid-binding-qualifier.frag
Signed-off-by: Tapani Pälli <tapani.palli@intel.com>
Reviewed-by: Samuel Iglesias Gonsálvez <siglesias@igalia.com>
When writing to a column of a row-major matrix, each component of the
vector is stored to non-consecutive memory addresses, so we generate
one instruction per component.
This patch skips the disabled components in the writemask, saving some
store instructions plus avoid storing wrong data on each disabled
component.
Signed-off-by: Samuel Iglesias Gonsalvez <siglesias@igalia.com>
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Fixes a failing subtest in:
ES31-CTS.shader_storage_buffer_object.negative-glsl-compileTime
Signed-off-by: Tapani Pälli <tapani.palli@intel.com>
Reviewed-by: Samuel Iglesias Gonsálvez <siglesias@igalia.com>
We may want to use different type_size functions for (e.g.) inputs
vs. uniforms. Passing in -1 for mode ignores this, handling all
modes as before.
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Matt Turner <mattst88@gmail.com>
From GLSL 1.50 spec, section 4.1.8 "Structures":
"Structures must have at least one member declaration."
So the base_alignment should be higher than zero.
Signed-off-by: Samuel Iglesias Gonsalvez <siglesias@igalia.com>
Reviewed-by: Ilia Mirkin <imirkin@alum.mit.edu>
From ARB_program_interface_query:
"For an active shader storage block member declared as an array, an
entry will be generated only for the first array element, regardless
of its type. For arrays of aggregate types, the enumeration rules are
applied recursively for the single enumerated array element."
v2:
- Simplify 'if' conditions and return true if it is not a buffer
variable, because these rules only apply to buffer variables (Timothy).
Signed-off-by: Samuel Iglesias Gonsalvez <siglesias@igalia.com>
Reviewed-by: Timothy Arceri <t_arceri@yahoo.com.au>
This matches the function signature created in
lower_ubo_reference_visitor::ssbo_store which has a void return.
Suggested-by: Jason Ekstrand <jason@jlekstrand.net>
Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
Reviewed-by: Samuel Iglesias Gonsálvez <siglesias@igalia.com>
At least on Intel hardware, gl_PrimitiveIDIn comes in as a special part
of the payload rather than a normal input. This is typically what we
use system values for. Dave and Ilia also agree that a system value
would be nicer.
At some point, we should change it at the GLSL IR level as well. But
that requires changing most of the drivers. For now, let's at least
make NIR do the right thing, which is easy.
v2: Add a comment about not creating a temporary (suggested by Iago).
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
The comment says that it should be impossible for decl_type to be NULL
here, so don't try to handle the case where it is, simply add an assert.
>>> CID 1324977: Null pointer dereferences (FORWARD_NULL)
>>> Comparing "decl_type" to null implies that "decl_type" might be null.
No piglit regressions observed.
Reviewed-by: Timothy Arceri <t_arceri@yahoo.com.au>
Add an assert on the result of as_dereference() not being NULL:
>>> CID 1324978: Null pointer dereferences (NULL_RETURNS)
>>> Dereferencing a null pointer "deref_record->record->as_dereference()".
Since we are introducing a new variable to hold the result of
as_dereference(), take the opportunity to rename deref_record_type to
interface_type and just name the new variable interface_deref, which is
less confusing.
Reviewed-by: Kristian Høgsberg <krh@bitplanet.net>
We don't use param in this part of the code, so no point in advancing
the pointer forward:
>>> CID 1324983: Code maintainability issues (UNUSED_VALUE)
>>> Assigning value from "param->get_next()" to "param" here, but that stored value is overwritten before it can be used.
Reviewed-by: Kristian Høgsberg <krh@bitplanet.net>
Because it counts shader storage blocks too.
v2:
- Use NumBufferInterfaceBlocks instead (Jordan).
Signed-off-by: Samuel Iglesias Gonsalvez <siglesias@igalia.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
Reviewed-by: Kristian Høgsberg <krh@bitplanet.net>
This reverts commit 586142658e.
The specs are not explicit about any restrictions related to the types allowed
on buffer variables, however, the description of opaque types (like atomic
counters) is in conclict with the purpose of buffer variables:
"The opaque types declare variables that are effectively opaque
handles to other objects. These objects are
accessed through built-in functions, not through direct reading or
writing of the declared variable.
(...)
Opaque variables cannot be treated as l-values;(...)"
Also, Mesa is already disallowing opaque types in interface blocks anyway, so
that commit was not really achieving anything.
Reviewed-by: Francisco Jerez <currojerez@riseup.net>
Some hardware (such as Broadwell) can run geometry shaders more
efficiently when the number of vertices emitted is statically known.
This pass provides a way to obtain the constant vertex count, or
-1 indicating that the vertex count is unknown/non-constant.
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
Commit 1ca25ab (glsl: Do not eliminate 'shared' or 'std140' blocks
or block members) considered as active 'shared' and 'std140' uniform
blocks and uniform block arrays, but did not include the block array
elements. Because of that, it was possible to have an active uniform
block array without any elements marked as used, making the assertion
((b->num_array_elements > 0) == b->type->is_array())
in link_uniform_blocks() fail.
Fixes the following 5 dEQP tests:
* dEQP-GLES3.functional.ubo.random.nested_structs_instance_arrays.18
* dEQP-GLES3.functional.ubo.random.nested_structs_instance_arrays.24
* dEQP-GLES3.functional.ubo.random.nested_structs_arrays_instance_arrays.19
* dEQP-GLES3.functional.ubo.random.all_per_block_buffers.49
* dEQP-GLES3.functional.ubo.random.all_shared_buffer.36
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=83508
Tested-by: Tapani Pälli <tapani.palli@intel.com>
Reviewed-by: Timothy Arceri <t_arceri@yahoo.com.au>
Reviewed-by: Kristian Høgsberg <krh@bitplanet.net>
Signed-off-by: Samuel Iglesias Gonsalvez <siglesias@igalia.com>
Reviewed-by: Tapani Pälli <tapani.palli@intel.com>
Reviewed-by: Kristian Høgsberg <krh@bitplanet.net>
Including TOP_LEVEL_ARRAY_SIZE and TOP_LEVEL_ARRAY_STRIDE queries.
v2:
- Use std430_array_stride() to get top level array stride following std430's rules.
Signed-off-by: Samuel Iglesias Gonsalvez <siglesias@igalia.com>
Reviewed-by: Tapani Pälli <tapani.palli@intel.com>
Reviewed-by: Kristian Høgsberg <krh@bitplanet.net>
