The spec is quite clear this is not allowed:
From Section 4.4. (Layout Qualifiers) of the GLSL 4.60 spec:
"Layout qualifiers can appear in several forms of declaration.
They can appear as part of an interface block definition or
block member, as shown in the grammar in the previous section.
They can also appear with just an interface-qualifier to establish
layouts of other declarations made with that qualifier:
layout-qualifier interface-qualifier ;
Or, they can appear with an individual variable declared with
an interface qualifier:
layout-qualifier interface-qualifier declaration ;"
From Section 4.10 (Memory Qualifiers) of the GLSL 4.60 spec:
"Layout qualifiers cannot be used on formal function parameters,
and layout qualification is not included in parameter matching."
However on the Nvidia binary driver they actually fail to compile
if image function params don't have a layout qualifier. This results
in applications such as No Mans Sky using layout qualifiers on params.
I've submitted a CTS test to expose this problem in the Nvidia driver
but until that is resolved this patch will help Mesa drivers work
around the issue.
Reviewed-by: Marek Olšák <marek.olsak@amd.com>
This fixes compilation of some "No Mans Sky" shaders where the stringification
happens in branches intended for DX12.
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
This commit expands the current memory access enum to contain the extra
two bits provided for images. We choose to follow the SPIR-V convention
of NonReadable and NonWriteable because readonly implies that you *can*
read so readonly + writeonly doesn't make as much sense as NonReadable +
NonWriteable.
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
The GLSL spec allows you to set both the "readonly" and "writeonly"
qualifiers on images to indicate that it can only be used with
imageSize. However, we had no way of representing this int he linked
shader and flagged it as GL_READ_ONLY. This is good from a "does it use
this buffer?" perspective but not from a format and access lowering
perspective. By using GL_NONE for if "readonly" and "writeonly" are
both set, we can detect this case in the driver and handle it correctly.
Nothing currently relies on the type of surface in the "readonly" +
"writeonly" case but that's about to change. i965 is the only drier
which uses the ImageAccess field and gl_bindless_image::access is
currently unused.
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Instead of requiring 4 components, this allows them to potentially use
fewer. Both the SPIR-V and GLSL paths still generate vec4 intrinsics so
drivers which assume 4 components should be safe. However, we want to
be able to shrink them for i965.
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
During intra stage linking some out variables can be dropped because
it is not used in a shader with the main function. But these out vars
can be referenced on later stages which can lead to further linking
errors.
Signed-off-by: Vadym Shovkoplias <vadym.shovkoplias@globallogic.com>
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105731
This extension provides new GLSL built-in function
beginFragmentShaderOrderingIntel() that guarantees
(taking wording of GL_INTEL_fragment_shader_ordering
extension) that any memory transactions issued by
shader invocations from previous primitives mapped to
same xy window coordinates (and same sample when
per-sample shading is active), complete and are visible
to the shader invocation that called
beginFragmentShaderOrderingINTEL().
One advantage of INTEL_fragment_shader_ordering over
ARB_fragment_shader_interlock is that it provides a
function that operates as a memory barrie (instead
of a defining a critcial section) that can be called
under arbitary control flow from any function (in
contrast the begin/end of ARB_fragment_shader_interlock
may only be called once, from main(), under no control
flow.
Signed-off-by: Kevin Rogovin <kevin.rogovin@intel.com>
Reviewed-by: Plamena Manolova <plamena.manolova@intel.com>
>From Section 4.3.4 (Inputs) of the GLSL 1.50 spec:
"Only the input variables that are actually read need to be written
by the previous stage; it is allowed to have superfluous
declarations of input variables."
Fixes:
* interstage-multiple-shader-objects.shader_test
v2:
Update comment in ir.h since the usage of "used" field
has been extended.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101247
Signed-off-by: Vadym Shovkoplias <vadym.shovkoplias@globallogic.com>
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
This reverts commit ae7898dfdb.
Turns out the python scripts are _not_ fully python 3 compatible.
As Ilia reported using get_xmlpool.py with LANG=C produces some weird
output - see the link for details.
Even though the issue was spotted with the autoconf build, it exposes a
genuine problem with the script (and lack of lang handling of the meson
build.)
https://lists.freedesktop.org/archives/mesa-dev/2018-August/203508.html
because the closed driver exposes it.
It's equivalent to ARB_gpu_shader_int64.
In this patch, I did everything the same as we do for ARB_gpu_shader_int64.
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
Pretty much all of the scripts are python2+3 compatible.
Check and allow using python3, while adjusting the PYTHON2 refs.
Note:
- python3.4 is used as it's the earliest supported version
- python3 chosen prior to python2
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Acked-by: Eric Engestrom <eric.engestrom@intel.com>
Just like the rest of the tree - these should be run either as part of
the build system check target, or at the very least with an explicitly
versioned python executable.
Fixes: db8cd8e367 ("glcpp/tests: Convert shell scripts to a python script")
Fixes: 97c28cb082 ("glsl/tests: Convert optimization-test.sh to pure python")
Fixes: 3b52d29227 ("glsl/tests: reimplement warnings-test in python")
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Dylan Baker <dylan@pnwbakers.com>
Reviewed-by: Eric Engestrom <eric.engestrom@intel.com>
This is a patch from me and a patch from Mathieu Bridon squashed
together.
Signed-off-by: Dylan Baker <dylan@pnwbakers.com>
Reviewed-by: Dylan Baker <dylan@pnwbakers.com>
Reviewed-by: Mathieu Bridon <bochecha@daitauha.fr>
We are testing the behaviour of a tool, for different input files, each
one using a different newline sequence. ('\n' on UNIX, '\r\n' on
Windows, …)
Unfortunately, when opening a file in text mode, Python 3 will by
default enable the "universal newlines" mode, which means it replaces
all the known newline sequences by '\n'.
This (usually useful) behaviour breaks the tests, which are specifically
trying to handle files with newline sequences different from '\n'.
Disabling the universal newlines mode fixes the tests.
However, to keep the script compatible with both Python 2 and 3, we must
use the io.open() function instead of the open() builtin, as the latter
only knows about the `newline` argument on Python 3.
Reviewed-by: Dylan Baker <dylan@pnwbakers.com>
Python 3 does not automatically convert from bytes to unicode strings
like Python 2 used to do.
This commit makes sure we pass unicode strings to difflib.unified_diff,
so that the script works on both Python 2 and 3.
Reviewed-by: Dylan Baker <dylan@pnwbakers.com>
v2: - explicitly decode the output of subprocesses
- handle bytes and string types consistently rather than relying on
python 2's coercion for bytes and ignoring them in python 3
v3: - explicitly set encode as well as decode
- python 2.7 and 3.x `bytes` instead of defining an alias
Reviewed-by: Mathieu Bridon <bochecha@daitauha.fr>
The main purpose for having NV_fragment_shader_interlock
extension is because that extension is also for GLES31 while
the ARB extension is for GL only.
Reviewed-by: Plamena Manolova <plamena.manolova@intel.com>
Accessing scalar constant as an array in function call or
initializer list triggered assert in get_array_element.
Examples:
func(0[0]);
vec2 t = { 0[0], 0 };
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107550
Signed-off-by: Danylo Piliaiev <danylo.piliaiev@globallogic.com>
Reviewed-by: Tapani Pälli <tapani.palli@intel.com>
After commit "nir: Use derefs in nir_lower_samplers"
(75286c2d08) assumes one deref for both
the texture and the sampler. However there are cases (on OpenGL, using
ARB_gl_spirv) where SPIR-V is not providing a sampler, like for
texture query levels ops. Although we could make spirv_to_nir to
provide a sampler deref for those cases, it is not really needed, and
wrong from the Vulkan point of view.
This patch fixes the following (borrowed) tests run on SPIR-V mode:
arb_compute_shader/execution/basic-texelFetch.shader_test
arb_gpu_shader5/execution/sampler_array_indexing/fs-simple-texture-size.shader_test
arb_texture_query_levels/execution/fs-baselevel.shader_test
arb_texture_query_levels/execution/fs-maxlevel.shader_test
arb_texture_query_levels/execution/fs-miptree.shader_test
arb_texture_query_levels/execution/fs-nomips.shader_test
arb_texture_query_levels/execution/vs-baselevel.shader_test
arb_texture_query_levels/execution/vs-maxlevel.shader_test
arb_texture_query_levels/execution/vs-miptree.shader_test
arb_texture_query_levels/execution/vs-nomips.shader_test
glsl-1.30/execution/fs-textureSize-compare.shader_test
v2: merge lower_tex_src_to_offset and calc_sampler_offsets together,
update texture/sampler index and texture_array_size directly on
lower_tex_src_to_offset (Jason)
v3: clarify one comment (Jason)
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
So they are not exposed through the introspection API.
It is worth to note that the number of hidden uniforms of GLSL linking
vs SPIR-V linking would be somewhat different due the differen order
of the nir lowerings/optimizations.
For example: gl_FbWposYTransform. This is introduced as part of
nir_lower_wpos_ytransform. On GLSL that is executed after the IR-based
linking. So that means that on GLSL the UniformStorage will not
include this uniform. With the SPIR-V linking, that uniform is already
present, but marked as hidden. So it will be included on the
UniformStorage, but as hidden.
One alternative would create a special how_declared for that case, but
seemed an overkill. Using hidden should be ok as far as it is used
properly.
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
Now that all the build scripts are compatible with both Python 2 and 3,
we can flip the switch and tell Meson to use the latter.
Since Meson already depends on Python 3 anyway, this means we don't need
two different Python stacks to build Mesa.
Signed-off-by: Mathieu Bridon <bochecha@daitauha.fr>
Reviewed-by: Eric Engestrom <eric.engestrom@intel.com>
Reviewed-by: Dylan Baker <dylan@pnwbakers.com>
Return ir_rvalue::error_value with ast_post_inc, ast_post_dec if
parser error was emitted previously. This way process_array_size
won't see bogus IR generated like with commit 9c676a6427.
Signed-off-by: Tapani Pälli <tapani.palli@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98699
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Instead of plain snprintf(). To fix the MSVC 2013 build.
Fixes: 6ff0c6f4eb ("gallium: move ddebug, noop, rbug, trace to auxiliary to improve build times")
Cc: Marek Olšák <marek.olsak@amd.com>
Cc: Brian Paul <brianp@vmware.com>
Cc: Roland Scheidegger <sroland@vmware.com>
Signed-off-by: Andres Gomez <agomez@igalia.com>
Reviewed-by: Brian Paul <brianp@vmware.com>
Fixes new piglit test:
tests/spec/glsl-1.20/execution/qualifiers/vs-out-conversion-int-to-float-vec4-index.shader_test
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
v2: ignore names on purpose, for consistency with other places where
we are doing the same (Alejandro)
v3: changes proposed by Timothy Arceri, implemented by Alejandro Piñeiro:
* Remove redundant 'struct active_xfb_varying'
* Update several comments, including spec quotes if needed
* Rename struct 'active_xfb_varying_array' to 'active_xfb_varyings'
* Rename variable 'array' to 'active_varyings'
* Replace one if condition for an assert (<MAX_FEEDBACK_BUFFERS)
* Remove BufferMode initialization (was already done)
v4: simplify output pointer handling (Timothy)
Signed-off-by: Neil Roberts <nroberts@igalia.com>
Signed-off-by: Alejandro Piñeiro <apinheiro@igalia.com>
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
These are copied from the from the corresponding values in
ir_variable. The intention is to eventually use them in a pure-NIR
linker.
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
Now that the elements version handles both cases, remove the
non-elements version.
Reviewed-by: Eric Anholt <eric@anholt.net>
Reviewed-by: Thomas Helland <thomashelland90@gmail.com>
Keep information in acp_entry whether the entry is full or not, and
use the ACP in more nodes when visiting the instructions:
- add_copy: write whole variables to the ACP state (regardless the
type).
- visit(ir_dereference_variable *): perform the propagation here if we have a
full candidate. Element-wise here doesn't apply because the mask
isn't available at this point.
- visit_leave(ir_assignment *): process beyond scalar and vector, as
the full variables might have other types.
Also import an improvement from opt_copy_propagation.cpp: if ir_call
is an intrinsic, we know the variables affected, so keep going.
v2: (all from Eric Anholt)
Describe how acp_entry attributes are used.
Don't do book-keeping to avoid adding repeated element to
the dsts in write_elements().
v3: Use _mesa_set_remove_key. (Thomas Helland)
Reviewed-by: Eric Anholt <eric@anholt.net>
Reviewed-by: Thomas Helland <thomashelland90@gmail.com>
Python 2 has a range() function which returns a list, and an xrange()
one which returns an iterator.
Python 3 lost the function returning a list, and renamed the function
returning an iterator as range().
As a result, using range() makes the scripts compatible with both Python
versions 2 and 3.
Signed-off-by: Mathieu Bridon <bochecha@daitauha.fr>
Reviewed-by: Eric Engestrom <eric.engestrom@intel.com>
Reviewed-by: Dylan Baker <dylan@pnwbakers.com>
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
In Python 2, iterators had a .next() method.
In Python 3, instead they have a .__next__() method, which is
automatically called by the next() builtin.
In addition, it is better to use the iter() builtin to create an
iterator, rather than calling its __iter__() method.
These were also introduced in Python 2.6, so using it makes the script
compatible with Python 2 and 3.
Signed-off-by: Mathieu Bridon <bochecha@daitauha.fr>
Reviewed-by: Eric Engestrom <eric.engestrom@intel.com>
Reviewed-by: Dylan Baker <dylan@pnwbakers.com>
Delegating constructors is a C++11 feature, so this was breaking when
compiling with C++98. Change the copy_propagation_state() calls that
used the convenience constructor to use a static member function
instead.
Since copy_propagation_state is expected to be heap allocated, this
change is a good fit.
Tested-by: Vinson Lee <vlee@freedesktop.org>
Reviewed-by: Matt Turner <mattst88@gmail.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107305
When handling 'if' in copy propagation elements, if a certain variable
was killed when processing the first branch of the 'if', then the
second would get any propagation from previous nodes.
x = y;
if (...) {
z = x; // This would turn into z = y.
x = 22; // x gets killed.
} else {
w = x; // This would NOT turn into w = y.
}
With the change, we let copy propagation happen independently in the
two branches and only then apply the killed values for the subsequent
code.
One example in shader-db part of shaders/unity/8.shader_test:
(assign (xyz) (var_ref col_1) (var_ref tmpvar_8) )
(if (expression bool < (swiz y (var_ref xlv_TEXCOORD0) )(constant float (0.000000)) ) (
(assign (xyz) (var_ref col_1) (expression vec3 + (var_ref tmpvar_8) ... ) ... )
)
(
(assign (xyz) (var_ref col_1) (expression vec3 lrp (var_ref col_1) ... ) ... )
))
The variable col_1 was replaced by tmpvar_8 in the then-part but not
in the else-part.
NIR deals well with copy propagation, so it already covered for the
missing ones that this patch fixes.
Reviewed-by: Eric Anholt <eric@anholt.net>
Instead of keeping multiple acp_entries in lists, have a single
acp_entry per variable. With this, the implementation of clone is more
convenient and now fully implemented. In the previous code, clone was
only partial.
Before this patch, each acp_entry struct represented a write to a
variable including LHS, RHS and a mask of what channels were written
to. There were two main hash tables, the first (lhs_ht) stored a list
of acp_entries per LHS variable, with the values available to copy for
that variable; the second (rhs_ht) was a "reverse index" for the first
hash table, so stored acp_entries per RHS variable.
After the patch, there's a single acp_entry struct per LHS variable,
it contains an array with references to the RHS variables per
channel. There now is a single hash table, from LHS variable to the
corresponding entry. The "reverse index" is stored in the ACP entry,
in the form of a set of variables that copy from the LHS. To make the
clone operation cheaper, the ACP entries are created on demand.
This should not change the result of copy propagation, a later patch
will take advantage of the clone operation.
v2: Add note clarifying how the hashtable is destroyed.
v3: (all from Eric Anholt)
Add remove_unused_var_from_dsts() function for reuse.
Remove from dsts as we go instead of clearing at the end.
Add clarifying comment to erase().
Reviewed-by: Eric Anholt <eric@anholt.net>
Separate higher level logic of visiting instructions and chosing when
to store and use new copy data from the datastructure holding the copy
propagation information. This will also make easier later patches that
change the structure.
v2: Remove empty destructor and clarify how hash tables are destroyed.
Reviewed-by: Eric Anholt <eric@anholt.net>
The "__inst" will contain the name used for the variable of type
"__type *". Parenthesis is not necessary as the name itself shouldn't
be an expression.
Fixes warning:
In file included from ../../src/mesa/main/mtypes.h:49,
from ../../src/intel/compiler/brw_compiler.h:30,
from ../../src/intel/compiler/brw_shader.h:29,
from ../../src/intel/compiler/brw_fs.h:31,
from ../../src/intel/compiler/brw_fs_cse.cpp:24:
../../src/intel/compiler/brw_fs_cse.cpp: In member function ‘bool fs_visitor::opt_cse_local(bblock_t*)’:
../../src/compiler/glsl/list.h:675:12: warning: unnecessary parentheses in declaration of ‘entry’ [-Wparentheses]
__type *(__inst); \
^
../../src/intel/compiler/brw_fs_cse.cpp:257:10: note: in expansion of macro ‘foreach_in_list_use_after’
foreach_in_list_use_after(aeb_entry, entry, &aeb) {
^~~~~~~~~~~~~~~~~~~~~~~~~
Reviewed-by: Anuj Phogat <anuj.phogat@gmail.com>
