According to the GLSL ES 3.20, GLSL 4.50, and GLSL 1.20 specs:
"To force all output variables to be invariant, use the pragma
#pragma STDGL invariant(all)
before all declarations in a shader."
Notably, this is only supposed to affect output variables. Furthermore,
"Only variables output from a shader can be candidates for invariance."
It looks like this has been wrong since we first supported the pragma in
2011 (commit 86b4398cd1).
Fixes dEQP-GLES2.functional.shaders.preprocessor.pragmas.pragma_fragment.
v2: Now that all cases are identical (other than compute shaders, which
have no output variables anyway), we can drop the switch statement
entirely. We also don't need the current_function == NULL check;
this was a hold over from when we had a single var_mode_out for both
function parameters and shader varyings, in the bad old days.
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Reviewed-by: Ilia Mirkin <imirkin@alum.mit.edu>
Fixes: 94d669b0d2 ("glsl: enforce fragment shader input restrictions in
GLSL ES 3.10")
Signed-off-by: Andreas Boll <andreas.boll.dev@gmail.com>
Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com>
Reviewed-by: Eric Anholt <eric@anholt.net>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
NIR does not have these instructions. TGSI and Mesa IR both implement
them using < and >=, repsectively. Removing them deletes a bunch of
code and means I don't have to add code to the SPIR-V generator for
them.
v2: Rebase on 2+ years of change... and fix a major bug added in the
rebase.
text data bss dec hex filename
8255291 268856 294072 8818219 868e2b 32-bit i965_dri.so before
8254235 268856 294072 8817163 868a0b 32-bit i965_dri.so after
7815339 345592 420592 8581523 82f193 64-bit i965_dri.so before
7813995 345560 420592 8580147 82ec33 64-bit i965_dri.so after
Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
Reviewed-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
Without the lexer changes, tests/glslparsertest/glsl2/tex_rect-02.frag
fails. Before this change, the parser would determine that
sampler2DRect is not a valid type because the call to
state->symbols->get_type() in ast_type_specifier::glsl_type() would
return NULL. Since ast_type_specifier::glsl_type() is now going to
return the glsl_type pointer that it received from the lexer, it doesn't
have an opportunity to generate an error.
text data bss dec hex filename
8255243 268856 294072 8818171 868dfb 32-bit i965_dri.so before
8255291 268856 294072 8818219 868e2b 32-bit i965_dri.so after
7815195 345592 420592 8581379 82f103 64-bit i965_dri.so before
7815339 345592 420592 8581523 82f193 64-bit i965_dri.so after
Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
Reviewed-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
Instead of generating a sequence like:
run_default = true;
if (i == 3) // some label that appears after default
run_default = false;
if (i == 4) // some label that appears after default
run_default = false;
...
if (run_default) {
...
}
generate something like:
run_default = !((i == 3) || (i == 4) || ...);
if (run_default) {
...
}
This eliminates one use of conditional assignment, and it enables the
elimination of another.
Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Previously the instruction stream was walked looking for comparisons
with case-label values. This should generate nearly identical code.
For at least fs-default-notlast-fallthrough.shader_test, the code is
identical.
This change will make later changes possible.
Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
The values being compared are scalars, so these are the same. While
I'm here, simplify the run_default condition to just deref the flag
(instead of comparing a scalar bool with true).
There is a bit of extra change in this patch. When constructing an
ir_binop_equal ir_expression, there is an assertion that the types are
the same. There is no such assertion for ir_binop_all_equal, so
passing glsl_type::uint_type with glsl_type::int_type was previously
fine. A bunch of the code motion is to deal with that.
Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
This happens to work now because ir_binop_all_equal is used. This
causes vector typed init-expressions to produce scalar Boolean values
after comparison.
The next commit changes ir_binop_all_equal to ir_binop_equal. Vector
typed init-expressions will then produce vector Boolean values, and, in
debug builds, the ir_assignment constructor will fail an assertion.
Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
It leads to surprising states with integer inputs and outputs on
vertex processing stages (e.g. geometry stages). Instead, rely on the
driver to choose smooth interpolation by default.
We still allow varyings to match when one stage declares it as smooth
and the other declares it without interpolation qualifiers.
Reviewed-by: Marek Olšák <marek.olsak@amd.com>
Tested-by: Dieter Nützel <Dieter@nuetzel-hh.de>
In GLSL ES 3.10 session 4.9 [Memory Access Qualifiers], it has the
following description:
"A variable could be qualified as both readonly and writeonly,
disallowing both read and write, but still be passed to
imageSize() to have the size queried.".
This is for image variable, but not for buffer variables.
According to https://github.com/KhronosGroup/OpenGL-API/issues/7 Khronos
intent is to allow both readonly and writeonly in buffer variables, and
as such it will update the GLSL specification.
This commit address this issue, and fixes:
KHR-GL{43,44,45}.shader_storage_buffer_object.basic-readonly-writeonly
KHR-GLES31.core.shader_storage_buffer_object.basic-readonly-writeonly
v2: set correctly fields[i] memory flags (Samuel Pitoiset).
Reviewed-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
After get_variable_being_redeclared() has been called, it is no longer
safe to access the original variable pointer, since its memory might have
been freed.
Since callers of this function should only be accessing the variable pointer
returned by the function, avoid potential bugs by re-assigning the
original variable pointer to the result of the function call,
making it impossible for the remaining code to access an invalid variable
pointer.
Reviewed-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
get_variable_being_redeclared() can delete the original variable
in a specific scenario. The code sets it to NULL after this so other
code in that same function doesn't try to access trashed memory after
the fact, however, the copy of that variable in the caller code
won't see any of this making it very easy to overlook.
Make the function a bit safer by taking a pointer to the original
variable so we can also make NULL the caller's pointer to the variable
if this function deletes it.
Reviewed-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
Since the original 'var' might have been deleted from this point forward.
Bugzila: https://bugs.freedesktop.org/show_bug.cgi?id=102685
Fixes: 51bf007d2c (glsl: Disallow unsized array of atomic_uint)
Reviewed-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
This was a bugfix to the spec addressed in OpenGL 4.5 (revision
7 of the spec) and there is a CTS test to check this.
Fixes:
KHR-GL45.shader_atomic_counters.negative-unsized-array
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
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 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>
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>
_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 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>
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>
The spec text cited above says you can't, but only the GLSL 3.00 (redefine
or overload) case was implemented.
Fixes dEQP scoping.invalid.redefine_builtin_fragment/vertex.
Reviewed-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Tested-by: Matt Turner <mattst88@gmail.com>
From the spec,
Arrays are allowed as arguments, but not as the return type. [...] The
return type can also be a structure if the structure does not contain
an array.
Fixes DEQP shaders.functions.invalid.return_array_in_struct_fragment.
v2: Spec cite wording change
Reviewed-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Tested-by: Matt Turner <mattst88@gmail.com>
From section 4.1.7 of the ARB_bindless_texture spec:
"Samplers may be declared as shader inputs and outputs, as uniform
variables, as temporary variables, and as function parameters."
From section 4.1.X of the ARB_bindless_texture spec:
"Images may be declared as shader inputs and outputs, as uniform
variables, as temporary variables, and as function parameters."
v3: - add spec comment
- update the glsl error message
Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
Reviewed-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
Yes, this is a bit hacky but we don't really have the choice.
Plain GLSL doesn't accept bindless samplers/images as l-values
while it's allowed when ARB_bindless_texture is enabled.
The default NULL parameter is because we can't access the
_mesa_glsl_parse_state object in few places in the compiler.
One is_lvalue(NULL) call is for IR validation but other checks
happen elsewhere, should be safe.
Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Reviewed-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
From section 4.3.4 of the ARB_bindless_texture spec
"(modify last paragraph, p. 35, allowing samplers and images as
fragment shader inputs) ... Fragment inputs can only be signed
and unsigned integers and integer vectors, floating point scalars,
floating-point vectors, matrices, sampler and image types, or
arrays or structures of these. Fragment shader inputs that are
signed or unsigned integers, integer vectors, or any
double-precision floating- point type, or any sampler or image
type must be qualified with the interpolation qualifier "flat"."
v3: - update spec comment formatting
Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
Reviewed-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
From section 4.3.4 of the ARB_bindless_texture spec:
"(modify third paragraph of the section to allow sampler and
image types) ... Vertex shader inputs can only be float,
single-precision floating-point scalars, single-precision
floating-point vectors, matrices, signed and unsigned integers
and integer vectors, sampler and image types."
v3: - update spec comment formatting
Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
Reviewed-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
From section 4.3.4 of the ARB_bindless_texture spec:
"(modify third paragraph of the section to allow sampler and image
types) ... Vertex shader inputs can only be float,
single-precision floating-point scalars, single-precision
floating-point vectors, matrices, signed and unsigned integers
and integer vectors, sampler and image types."
From section 4.3.6 of the ARB_bindless_texture spec:
"Output variables can only be floating-point scalars,
floating-point vectors, matrices, signed or unsigned integers or
integer vectors, sampler or image types, or arrays or structures
of any these."
v3: - add spec comment
Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
Reviewed-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
The spec doesn't clearly state this, but I have got clarification
from the spec authors.
Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
Reviewed-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
From section 4.3.7 of the ARB_bindless_texture spec:
"(remove the following bullet from the last list on p. 39, thereby
permitting sampler types in interface blocks; image types are also
permitted in blocks by this extension)"
* sampler types are not allowed
v3: - update the spec comment
- update the glsl error message
Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
Reviewed-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
The ARB_bindless_texture spec doesn't clearly state this, but as
it says "Replace Section 4.1.7 (Samplers), p. 25" and,
"Replace Section 4.1.X, (Images)", this should be allowed.
v3: - add spec comment
- update the glsl error message
Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
Reviewed-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
From section 4.1.7 of the ARB_bindless_texture spec:
"Samplers can be used as l-values, so can be assigned into and used
as "out" and "inout" function parameters."
From section 4.1.X of the ARB_bindless_texture spec:
"Images can be used as l-values, so can be assigned into and used as
"out" and "inout" function parameters."
v3: - add spec comment
- update the glsl error message
Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
Reviewed-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
From section 4.1.7 of the ARB_bindless_texture spec:
"Samplers may be declared as shader inputs and outputs, as uniform
variables, as temporary variables, and as function parameters."
From section 4.1.X of the ARB_bindless_texture spec:
"Images may be declared as shader inputs and outputs, as uniform
variables, as temporary variables, and as function parameters."
v3: - add validate_storage_for_sampler_image_types()
- update spec comment
- update the glsl error message
Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
Reviewed-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
This adds bindless_sampler and bound_sampler (and respectively
bindless_image and bound_image) to the parser.
v3: - add an extra space in apply_bindless_qualifier_to_variable()
- fix indentation in merge_qualifier()
Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
Reviewed-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
In plain GLSL, sampler and image types can only be declared
uniform-qualified global variables or 'in' function parameters.
Setting the read_only flag seems quite useless because other
checks will prevent sampler/image variables to be assigned and
also because the flag is not set for atomic_uint types which are
opaque types.
This will also help for ARB_bindless_texture because samplers
and images can be assigned when they are considered bindless.
Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Reviewed-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
The spec allows memory qualifiers to be used with image variables,
buffers variables and shader storage blocks. This patch also fixes
validate_memory_qualifier_for_type().
Fixes the following ARB_uniform_buffer_object test:
uniform-block-memory-qualifier.frag
Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Reviewed-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
Including structures, interfaces and uniform blocks.
Fixes the following ARB_shader_image_load_store test:
format-layout-with-non-image-type.frag
Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Reviewed-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
It makes more sense to have two separate validate functions,
mainly because memory qualifiers are allowed with members of
shader storage blocks.
validate_memory_qualifier_for_type() will be fixed in a
separate patch.
Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Reviewed-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
It doesn't make sense to prefix them with 'image' because
they are called "Memory Qualifiers" and they can be applied
to members of storage buffer blocks.
Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Reviewed-by: Andres Gomez <agomez@igalia.com>
