We continue in the code to do some more things with the rhs, including
setting a constant initializer. If the type is wrong, this causes some
confusion down the line, leading to assertions. This makes sure that the
rhs processing continues to flow as-if the type was correct to start
with (even though the state has been marked as an error state).
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101766
Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Cc: mesa-stable@lists.freedesktop.org
The cloning was introduced in f81ede4699 to fix a problem with
shaders including IR that was owned by builtins.
However the approach of cloning the whole function each time we
reference a builtin lead to a significant reduction in the GLSL
IR compilers performance.
The previous patch fixes the ownership problem in a more precise
way. So we can now remove this cloning.
Testing on a Ryzen 7 1800X shows a ~15% decreases in compiling the
Deus Ex: Mankind Divided shaders on radeonsi (which take 5min+ on
some machines). Looking just at the GLSL IR compiler the speed up
is ~40%.
Tested-by: Dieter Nützel <Dieter@nuetzel-hh.de>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
The main motivation for this is that threaded compilation can fall
over if we were to allocate IR inside constant_expression_value()
when calling it on a builtin. This is because builtins are shared
across the whole OpenGL context.
f81ede4699 worked around the problem by cloning the entire
builtin before constant_expression_value() could be called on
it. However cloning the whole function each time we referenced
it lead to a significant reduction in the GLSL IR compiler
performance. This change along with the following patch
helps fix that performance regression.
Other advantages are that we reduce the number of calls to
ralloc_parent(), and for loop unrolling we free constants after
they are used rather than leaving them hanging around.
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
The Deus Ex: Mankind Divided shaders go from spending ~20 seconds
in the GLSL IR compilers front-end down to ~18.5 seconds on a
Ryzen 1800X.
Tested by compiling once with shader-db then deleting the index file
from the shader cache and compiling again.
v2:
- fix rebasing issue in v1
Reviewed-by: Thomas Helland <thomashelland90@gmail.com>
We are currently copying the name for each member dereference
but we can just share a single instance of the string provided
by the type.
This change also stops us recalculating the field index
repeatedly.
Reviewed-by: Thomas Helland <thomashelland90@gmail.com>
Also add a comment that this should only be used by the ir_reader
interface for testing purposes.
v2:
- fix grammar in comment
- use unreachable rather than assert
Reviewed-by: Thomas Helland <thomashelland90@gmail.com>
Extra validation is added to ir_validate to make sure this is
always updated to the correct numer of operands, as passes like
lower_instructions modify the instructions directly rather then
generating a new one.
The reduction in time is so small that it is not really
measurable. However callgrind was reporting this function as
being called just under 34 million times while compiling the
Deus Ex shaders (just pre-linking was profiled) with 0.20%
spent in this function.
v2:
- make num_operands a unit8_t
- fix unsigned/signed mismatches
Reviewed-by: Thomas Helland <thomashelland90@gmail.com>
Other ones are either unsupported or don't have any helper
function checks.
Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Reviewed-by: Ilia Mirkin <imirkin@alum.mit.edu>
Check if shaders have transform feedback varyings also after the
post-link step.
This fixes:
KHR-GL45.enhanced_layouts.xfb_vertex_streams
piglit/spec/arb_enhanced_layouts/gs-stream-location-aliasing
v2: add claryfing comments (Timothy)
Reviewed-by: Marek Olšák <marek.olsak@amd.com>
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
When we have an interface block like:
layout (xfb_buffer = 0, xfb_offset = 0) out Block {
vec4 var1;
layout (xfb_stride = 48) vec4 var2;
vec4 var3;
};
According to ARB_enhanced_layouts spec:
"The *xfb_stride* qualifier specifies how many bytes are consumed by
each captured vertex. It applies to the transform feedback buffer
for that declaration, whether it is inherited or explicitly
declared. It can be applied to variables, blocks, block members, or
just the qualifier out. [ ...] While *xfb_stride* can be declared
multiple times for the same buffer, it is a compile-time or
link-time error to have different values specified for the stride
for the same buffer."
This means xfb_stride actually applies to the buffer, and not to the
individual components.
In the above example, it means that var2 consumes 16 bytes, and var3 is
at offset 32.
This has been confirmed also by John Kessenich, the main contact for the
ARB_enhanced_layouts specs, and also because this commit fixes:
GL45.enhanced_layouts.xfb_block_member_stride
This commit is in practice a revert of 598790e856 (glsl: apply
xfb_stride to implicit offsets for ifc block members).
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
Reviewed-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
From the ARB_uniform_buffer_object spec:
""shared" uniform blocks, the default layout, ..."
This doesn't fix anything as the default layout is already applied
at this point but fixes the misleading code/comment.
Reviewed-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
This was added in 2d03f48a65 and seems like it was intended
as a TODO comment in a function stub rather than a useful
code comment.
Reviewed-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
These are intrinsics rather than opcodes, because they operate across
channels.
Reviewed-by: Connor Abbott <cwabbott0@gmail.com>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
While it produces functioning code the pass creates worse code
for arrays of arrays. See the comment added in this patch for more
detail.
V2: skip splitting of AoA of matrices too.
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
_mesa_glsl_has_builtin_function is used to determine whether any variant
of a builtin are available, for the purpose of enforcing the GLSL ES
3.00+ rule that overloads or overrides of builtins are disallowed.
However the builtin_builder contains information on all builtins,
irrespective of parse state, or versions, or extension enablement. As a
result we would say that a builtin existed even if it was not actually
available.
To resolve this, first check if at least one signature is available for
a builtin before returning true.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101666
Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
Cc: mesa-stable@lists.freedesktop.org
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
Acked-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
This is convenient for backends that support both Vulkan and OpenGL while
lowering samplers to derefs with nir_lower_samplers_as_deref.
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
Undefined data will eventually trigger a valgrind error while computing
its CRC32 while writing it into the disk cache, but at that point, it is
basically impossible to track down where the undefined data came from.
With this change, finding the origin of undefined data becomes easy.
v2: remove duplicate VALGRIND_CFLAGS (Emil)
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
Otherwise, the padding bits remain undefined, which leads to valgrind
errors when storing the gl_shader_variable in the disk cache.
v2: use rzalloc instead of an explicit padding member variable
Reviewed-by: Marek Olšák <marek.olsak@amd.com>
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
Save some passes over the IR.
v2: redesign to make the users of find_assignments more readable
v3:
- fix missing !
- add some comments and make the num_found check more explicit (Timothy)
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com> (v1)
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
We always have stage == first and stage == last when first == last, so
drop the special case. Also rephrase the comment to make the logic
clearer.
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
xfb only applies to the latest stage before the fragment shader, so
there is no need to invoke it in the fragment shader.
Fixes:
KHR-GL45.enhanced_layouts.xfb_stride_of_empty_list
KHR-GL45.enhanced_layouts.xfb_stride_of_empty_list_and_api
v2: do reset only if shaders provide an explicit stride
v3: do not call link_xfb_stride_layout_qualifiers() for fragment shaders
(Timothy)
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
Signed-off-by: Juan A. Suarez Romero <jasuarez@igalia.com>
The current implementation assumed that these were replaced in GLSL >= 4.10
by gl_Max{Vertex,Fragment}UniformVectors, however this is not true: both
built-ins should be produced from GLSL 4.10 onwards.
This was raised by new CTS tests that are in development.
Reviewed-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
Bindless sampler/image handles are represented using 64-bit
unsigned integers.
The ARB_bindless_texture spec says:
"The error INVALID_OPERATION is generated by UniformHandleui64{v}ARB
if the sampler or image uniform being updated has the "bound_sampler"
or "bound_image" layout qualifier"."
Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Reviewed-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
Unnamed struct types are now equal across stages based on the fields they
contain, so overriding the type to make sure names match has become
unnecessary.
The check was originally introduced in commit 955c93dc08 ("glsl: Match
unnamed record types across stages.")
v2: clarify the commit message
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
As a result, unnamed structs defined in different places of the program
are considered the same types if they have the same fields in the same
order.
This will simplify matching of global variables whose type is an unnamed
struct.
It also fixes a memory leak when the same shader containing unnamed
structs is compiled over and over again: instead of creating a new type
each time, the existing type is re-used.
Finally, this does have the effect that some previously rejected programs
are now accepted, such as:
struct {
float a;
} s1;
struct {
float a;
} s2;
s2 = s1;
C/C++ do not allow that, but GLSL does seem to want to treat unnamed
structs with the same fields as the same type at least during linking
(and apparently, some applications require it), so it seems odd to treat
them as different types elsewhere.
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
This changes the logic during the conversion of the declaration list
struct S {
...
} v;
from AST to IR, but should not change the end result.
When assigning the type of v, instead of looking `S' up in the symbol
table, we read the type from the member variable of ast_struct_specifier.
This change is necessary for the subsequent change to how anonymous types
are handled.
v2: remove a type override when redefining a structure; should be
the same type in that case anyway
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
The max_array_access field applies to the first dimension, which means
we only want to set it for the 1D clip dist arrays.
This fixes an ir_validate assert seen with
KHR-GL44.cull_distance.functional
on nouveau and radeon with debug builds.
Fixes: a08c4ebbe (glsl: rewrite clip/cull distance lowering pass)
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
Tested-by: Tobias Klausmann <tobias.johannes.klausmann@mni.thm.de>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Replace -1 with MESA_SHADER_NONE enum value to fix sign related warning:
external/mesa3d/src/compiler/glsl/link_varyings.cpp:1415:25: warning: comparison of constant -1 with expression of type 'gl_shader_stage' is always true [-Wtautological-constant-out-of-range-compare]
(consumer_stage != -1 && consumer_stage != MESA_SHADER_FRAGMENT))) {
~~~~~~~~~~~~~~ ^ ~~
Reviewed-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
Signed-off-by: Rob Herring <robh@kernel.org>
