We were accidentally asserting that the value had to be a vtn_ssa_value
which isn't true if it, for instance, comes from a spec constant.
Fixes: fb282a68bc "spirv: Implement OpConvertPtrToU and OpConvertUToPtr"
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
Acked-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4675>
After the decorations of a variable are evaluated, propagate the
access flag to the associated vtn_pointer. This was done when
creating the pointer but at that point there was no access flags for
the variable.
Inline the pointer creation to make this point clearer, in isolation
the helper made the impression that the value was being propagated.
Issue found by Ken.
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4620>
This also makes spirv_to_nir a bit simpler because the new
nir_vector_insert helper automatically handles a constant component
selector like nir_vector_extract does.
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4495>
The nir_builder helper already handles checking if the component
selector is an immediate and returns an undef in the OOB case.
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4495>
This adds support for OpVariable having an initializer that points to
another variable, rather than a constant. In this case, the variable is
initialized to a pointer to the other variable.
Fixes Vulkan CTS tests:
dEQP-VK.spirv_assembly.instruction.compute.variable_init.private.*
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/merge_requests/3047>
The caller is responsible for setting up the ubo_addr_format value as
contrary to shared and global, it's not controlled by the spirv.
Right now clovers implementation of CL constant memory uses a 24/8 bit format
to encode the buffer index and offset, but that code is dead as all backends
treat constants as global memory to workaround annoying issues within OpenCL.
Maybe that will change, maybe not. But just in case somebody wants to look at
it, add a toggle for this inside vtn.
Signed-off-by: Karol Herbst <kherbst@redhat.com>
Reviewed-by: Dave Airlie <airlied@redhat.com>
Fixes: 9cc4c2c916 "spirv: Add a vtn_decorate_pointer helper"
Tested-by: Erik Faye-Lund <erik.faye-lund@collabora.com>
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
This commit makes two major changes. First, we add a second case to
OpLoad for sampled images which constructs a vtn_sampled_image and
stashes that rather than stashing a pointer to the combined image
sampler like we do for bare samplers and images. This should be more in
line with how SPIR-V is intended to work and hopefully doesn't cause any
weird problems. The second is a rework of vtn_handle_texture to assume
that everything has an image but not everything has a sampler. We also
add a vtn_fail_if for the case where a texture instructions require a
sampler but none is provided.
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
This helper makes a duplicate copy of the pointer if any new access
flags are set at this stage. This way we don't end up propagating
access flags further than they actual SPIR-V decorations. In several
instances where we create new pointers, we still call the decoration
helper directly because no copy is needed.
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
We have types on all vtn_values at this point so there's no reason to
carry the redundant type information.
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
Emit barriers with semantics matching the access operand and the
storage class of the pointer.
v2: Fix order of visible / available emission relative to the
operations. (Bas)
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
Intel drivers are not using this anymore, and turnip still don't have
Compute Shaders, so won't make a difference to stop using this option.
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Acked-by: Rob Clark <robdclark@chromium.org>
Lowers BaseVertex to the correct system value for OpenGL.
v2: use options->environment rather than adding a new flag to
spirv_to_nir_options
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
We can have a access flag already set here so just augment the
existing ones.
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Fixes: 0fb61dfdeb ("spirv: propagate access qualifiers through ssa & pointer")
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
Not only variables can be flagged as NonUniformEXT but also
expressions. We're currently ignoring it in an expression such as :
imageLoad(data[nonuniformEXT(rIndex)], 0)
The associated SPIRV :
OpDecorate %69 NonUniformEXT
...
%69 = OpLoad %61 %68
This changes propagates access qualifiers through ssa & pointers so
that when it hits a OpLoad/OpStore style instructions, qualifiers are
not forgotten.
Fixes failure the following tests :
dEQP-VK.descriptor_indexing.*
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Fixes: 8ed583fe52 ("spirv: Handle the NonUniformEXT decoration")
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
This refactor allows for common code to apply decoration on all
ssa/pointer values. In particular this will allow to propagage access
qualifiers.
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Suggested-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
This decoration can be ignored, so we can just skip the next steps.
Otherwise we'd have to also handle it in apply_var_decoration.
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
On AMD, FragCoord should be a sysval because it is handled separately
from all the other inputs. We were already doing this in radeonsi, but
we weren't doing it with radv. It'll be much more annoying to handle
VARYING_SLOT_POS in fragment shaders when we let NIR lower FS inputs for
us, so here we add an option so that radv can get it as a system value.
Reviewed-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
The data for some nir variables is only filled up for some specific
modes. We need now too for UBO/SSBO, as such info would be used when
linking for OpenGL (ARB_gl_spirv).
There is an existing comment just before that code (starts with XXX)
that points that binding still needs to be filled up for uniform
variables at that point, and that should be fixed, although it doesn't
specify why that's a problem or what would be the alternative. For now
doing the same for UBO/SSBO, and will hope that the future fixing is
done for all of them.
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
Providing nir variables for UBO/SSBO it is not required for Vulkan,
but it is needed for OpenGL (ARB_gl_spirv), like for example, to
gather info from the UBO/SSBO while linking.
In opposite with most cases where the nir variables is created, here
the type assigned is the full type (not just the bare type). This is
needed because while linking using the nir shader we need the explicit
layout info (explicit stride, explicit offset, row_major, etc).
Also, we need to assign an interface type, used also on the OpenGL
linker if it is a UBO/SSBO. See ir_variable::is_in_buffer_block as
example.
v2: assign interface_type to be the variable type, not need to be
arrayness (Timothy)
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
In ARB_gl_spirv we'll be able to use variables for uniform buffers, so
don't use the descriptor intrinsics to lower the block access.
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
When creating function parameters, we create pointers from ssa
values, this creates nir casts with stride 0, however we have
no where else to get this value from. Later passes to lower
explicit io need this stride value to do the right thing.
Reviewed-by: Karol Herbst <kherbst@redhat.com>
SPV_GOOGLE_decorate_string and SPV_GOOGLE_hlsl_functionality1 were
incorporated to SPIR-V. Let's pick the names used by SPIR-V core.
Reviewed-by: Karol Herbst <kherbst@redhat.com>
And change vtn_storage_class_to_mode() to accept NULL as
interface_type. In this case, if we have a SpvStorageClassUniform, we
assume it is uses an ubo_addr_format, like the code being replaced by
the helper.
That assumption is a problem, but no different than the previous
code.
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
Corresponding to SpvStorageClassImage. We see pointers for that
storage class in tests, but don't use the storage class any further.
Adding this so that we can call vtn_mode_to_address_format() for all
supported pointers.
v2: Fail when trying to create a SpvStorageClassImage
variable. (Jason)
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
Handles all the modes and we can use it in combination with
nir_address_format_to_glsl_type() to replace the
vtn_ptr_type_for_mode() helper. Since the new helper is more generic,
moved the assertions from the old one to the call sites.
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
Just the mode is needed to decide whether SSA offsets are needed, so
make a function that takes that and reuse it for
vtn_pointer_uses_ssa_offset().
This will be used for constant null pointers, that won't have a
vtn_pointer handy.
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
v2: Renamed from vtn_interface_type. (Jason)
Accept any type not only pointers.
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
Instead of setting the glsl types of the pointers for each resource,
set the nir_address_format, from which we can derive the glsl_type,
and in the future the bit pattern representing a NULL pointer.
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
Decorations (and ExecutionModes) can have not only literals, but also
Ids associated with them. So rename the field to the more general
name "Operand" used by the spec.
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
v2: When available, include the opcode name too. (Karol)
v3: Use more to_string helpers. (Karol)
Include the wrong bit_size in those failures.
Include the capability number in spv_check_supported.
Provide vtn_fail_with_* macros to avoid noise in the call sites.
v4: Provide macros only for opcode and decoration, which have enough
usages to justify them. (Jason)
Acked-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Karol Herbst <kherbst@redhat.com>
v2: remove & operator in a couple of memsets
add some memsets
v3: fixup lima
Signed-off-by: Karol Herbst <kherbst@redhat.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net> (v2)
Otherwise nir_lower_non_uniform_access crashes when it tries
to get the access of a load_ubo.
Fixes: 8ed583fe52 "spirv: Handle the NonUniformEXT decoration"
Fixes: e50ab2c0f2 "nir: Add access flags to deref and SSBO atomics"
Reviewed-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
It was only propagated when UBO/SSBO access are lowered to offsets.
Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Reviewed-by: <Jason Ekstrand jason@jlekstrand.net>
Also handle GLSL_TYPE_INTERFACE the same way we do GLSL_TYPE_STRUCT in
various places. Motivated by ARB_gl_spirv work, that will take
advantage of the interface types when handling NIR coming from SPIR-V.
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
For OpenCL we never want to strip the info from the types, and it makes
type comparisons easier in later stages. We might later need a nir pass to
strip this for GLSL, but so far the only regression is the assert and Jason
said removing that is fine.
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Unlike most of the cases in which we do this by hand, the new helper
properly handles non-32-bit pointers.
Reviewed-by: Karol Herbst <kherbst@redhat.com>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Because we already know the immediate right-hand parameter, we can
potentially save the optimizer a bit of work.
Reviewed-by: Karol Herbst <kherbst@redhat.com>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
v2: use formula with fewer operations
Signed-off-by: Karol Herbst <kherbst@redhat.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
We need more space than just a 32-bit scalar and we have to burn all
that space anyway so we may as well expose it to the driver. This also
fixes a subtle bug when UBOs and SSBOs have different pointer types.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
With the new deref changes, the old pointer_offset version may not be
the right one to call. Just call the generic one and let it sort it
out.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
We can't pull it from the variable type because it might be an array of
blocks and not just the one block. While we're here, throw in some
error checking.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: mesa-stable@lists.freedesktop.org
On GLSL that info is set as a layout qualifier when redeclaring
gl_FragCoord, so somehow tied to a specific variable. But in practice,
they behave as a global of the shader. On ARB programs they are set
using a global OPTION (defined at ARB_fragment_coord_conventions), and
on SPIR-V using ExecutionModes, that are also not tied specifically to
the builtin.
This patch moves that info from nir variable and ir variable to nir
shader and gl_program shader_info respectively, so the map is more
similar to SPIR-V, and ARB programs, instead of more similar to GLSL.
FWIW, shader_info.fs already had pixel_center_integer, so this change
also removes some redundancy. Also, as struct gl_program also includes
a shader_info, we removed gl_program::OriginUpperLeft and
PixelCenterInteger, as it would be superfluous.
This change was needed because recently spirv_to_nir changed the order
in which execution modes and variables are handled, so the variables
didn't get the correct values. Now the info is set on the shader
itself, and we don't need to go back to the builtin variable to set
it.
Fixes: e68871f6a ("spirv: Handle constants and types before execution
modes")
v2: (Jason)
* glsl_to_nir: get the info before glsl_to_nir, while all the rest
of the info gathering is happening
* prog_to_nir: gather the info on a general info-gathering pass,
not on variable setup.
v3: (Jason)
* Squash with the patch that removes that info from ir variable
* anv: assert that OriginUpperLeft is true. It should be already
set by spirv_to_nir.
* blorp: set origin_upper_left on its core "compile fragment
shader", not just on some specific places (for this we added an
helper on a previous patch).
* prog_to_nir: no need to gather specifically this fragcoord modes
as the full gl_program shader_info is copied.
* spirv_to_nir: assert that we are a fragment shader when handling
this execution modes.
v4: (reported by failing gitlab pipeline #18750)
* state_tracker: update too due changes on ir.h/gl_program
v5:
* blorp: minor change after change on previous patch
* radeonsi: update due this change.
v6: (Timothy Arceri)
* prog_to_nir: remove extra whitespace
* shader_info: don't use :1 on origin_upper_left
* glsl: program.fs.origin_upper_left/pixel_center_integer can be
move out of the shader list loop
nir_lower_clip_cull_distance_arrays() marks the combined clip/cull
distance array as compact. However, when translating in from GLSL
or SPIR-V, we were not marking the original float[] arrays as compact.
We should do so. That way, we can detect these corner cases properly.
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
Under Vulkan, the double vertex attributes take up the same size
regardless of whether they are vertex inputs or any other stage
interface.
Under OpenGL (ARB_gl_spirv), from GLSL 4.60 spec, section 4.3.9
Interface Blocks:
"It is a compile-time error to have an input block in a vertex
shader or an output block in a fragment shader. These uses are
reserved for future use."
So we also don't need to check if it is an vertex input or not, and
use false in any case.
v2: (changes made by Alejandro Piñeiro)
* Update required after "spirv: Handle location decorations on
block interface members" own updates (original patch was sent
several months ago)
* After Neil suggesting it, confirm that this change can be also
done for OpenGL (ARB_gl_spirv). Expand commit message.
v3: update after changing name of main method on a previous patch
Signed-off-by: Neil Roberts <nroberts@igalia.com>
Signed-off-by: Alejandro Piñeiro <apinheiro@igalia.com>
Reviewed-by: Tapani Pälli <tapani.palli@intel.com>
Previously the code was taking any location decoration on the block
and using that to calculate the member locations for all of the
members. I think this was assuming that there would only be one
location decoration for the entire block. According to the Vulkan spec
it is possible to add location decorations to individual members:
“If the structure type is a Block but without a Location, then each
of its members must have a Location decoration. If it is a Block
with a Location decoration, then its members are assigned
consecutive locations in declaration order, starting from the
first member which is initially the Block. Any member with its own
Location decoration is assigned that location. Each remaining
member is assigned the location after the immediately preceding
member in declaration order.”
This patch makes it instead keep track of which members have been
assigned an explicit location. It also has a space to store the
location for the struct as a whole. Once all the decorations have been
processed it iterates over each member to fill in the missing
locations using the rules described above.
So, this commit is needed to get working a case like this, on both
Vulkan and OpenGL using SPIR-V (ARB_gl_spirv):
out block {
layout(location = 2) vec4 c;
layout(location = 3) vec4 d;
layout(location = 0) vec4 a;
layout(location = 1) vec4 b;
} name;
v2: (changes made by Alejandro Piñeiro)
* Update after introducing struct member splitting (See commit b0c643d)
* Update after only exposing interface_type for blocks, not to any struct
* Update after last changes done for xfb support
v3: use "assign" instead of "add" on the new method added (Tapani)
Signed-off-by: Neil Roberts <nroberts@igalia.com>
Signed-off-by: Alejandro Piñeiro <apinheiro@igalia.com>
Reviewed-by: Tapani Pälli <tapani.palli@intel.com>
This only implements the actual opcodes and does not implement support
for using them with specialization constants.
Reviewed-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
This was valid back when the only valid types of pointers were uint32
and uvec2. Now that we're allowing more variety, it could be just about
anything so we'll just drop the assert.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
Reviewed-by: Karol Herbst <kherbst@redhat.com>
Instead of setting interface_type to whatever the per-vertex type is, we
only set it on blocks. This allows later passes to tell the difference
between variables that are in blocks and those that aren't.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Instead of splitting every per-vertex struct, just split the ones that
are actually blocks. The reason for the split is so that we have
separate variables for separate locations, qualifiers, and builtin
decorations. The vulkan spec only allows these on members of blocks.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
vtn supports these, so don't squalk if user is happy with enabling
these.
v2: add new members sorted
Signed-off-by: Karol Herbst <kherbst@redhat.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
v2: rename nir_var_global to nir_var_mem_global
Signed-off-by: Karol Herbst <kherbst@redhat.com>
Acked-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Eric Anholt <eric@anholt.net>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
Signed-off-by: Karol Herbst <kherbst@redhat.com>
Acked-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Eric Anholt <eric@anholt.net>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
Signed-off-by: Karol Herbst <kherbst@redhat.com>
Acked-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Eric Anholt <eric@anholt.net>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
Signed-off-by: Karol Herbst <kherbst@redhat.com>
Acked-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Eric Anholt <eric@anholt.net>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
Signed-off-by: Karol Herbst <kherbst@redhat.com>
Acked-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Eric Anholt <eric@anholt.net>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
Signed-off-by: Karol Herbst <kherbst@redhat.com>
Acked-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Eric Anholt <eric@anholt.net>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
Instead of applying the workaround universally, detect semi-old GLSLang
via the generator ID and only enable the workaround on old GLSLang.
This isn't nearly as precise as one would like it to be because the
first GLSLang generator id version bump was on October 7, 2017 which is
about 1.5 years after the bug was fixed. However, it at least lets us
disable it for non-GLSLang and for more modern versions.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
A long time in a galaxy far far away, there was a GLSLang bug with how
it handled samplers passed in as function parameters. (The bug can be
found here: https://github.com/KhronosGroup/glslang/issues/179.)
Unfortunately, that version was shipped in several apps and has been
causing heartburn for our SPIR-V parser ever since.
Recent changes to NIR uncovered a moderately old bug in how we work
around this issue. In particular, we ended up with a deref_cast from
uniform to local which is not a no-op cast so nir_opt_deref wasn't
getting rid of the cast. The only reason why it worked before was
because someone just happened to call nir_fixup_deref_modes which
"fixed" the cast (that shouldn't be happening) and then a later round of
copy-prop would get rid of it. The fact that the deref_cast survived
that long without causing trouble for other parts of NIR is a bit
surprising.
Just whacking the mode of the pointer seems to fix it fairly
unobtrusively. Currently, only apps with this bug will have a local
variable containing an image or sampler.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109304
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
the naming is a bit confusing no matter how you look at it. Within SPIR-V
"global" memory is memory accessible from all threads. glsl "global" memory
normally refers to shader thread private memory declared at global scope. As
we already use "shared" for memory shared across all thrads of a work group
the solution where everybody could be happy with is to rename "global" to
"private" and use "global" later for memory usually stored within system
accessible memory (be it VRAM or system RAM if keeping SVM in mind).
glsl "local" memory is memory only accessible within a function, while SPIR-V
"local" memory is memory accessible within the same workgroup.
v2: rename local to function as well
v3: rename vtn_variable_mode_local as well
Signed-off-by: Karol Herbst <kherbst@redhat.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
For now, it's hidden behind a cap. Hopefully, we can eventually drop
that along with all the manual offset code in spirv_to_nir.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
Tested-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
The choice of whether or not we should use block_load/store isn't a
choice between external and not so much as a choice between deref
instructions and manually calculated offsets. In vtn_pointer_from_ssa,
we guard the index+offset case behind vtn_pointer_uses_ssa_offset and
then branch out from there.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
Previously, we hard-coded the rule about workgroup variables and the
builder lower_workgroup_access_to_offsets flag. Instead base it on the
handy helper we have for exactly this sort of thing.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
Variable pointers being well-defined across the block boundary requires
a couple of very specific SPIR-V validation rules. Normally, we'd trust
the validator to catch these but since CTS tests have been found in the
wild which violate them, we'll carry our own checks.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
These correspond directly to SPIR-V's OpPtrAccessChain. As such, they
treat whatever their parent gives them as if it's the first element in
some array and dereferences that array. If the parent is, itself, an
array deref, then the two indices can just be added together to get the
final array deref. However, it can also be used in cases where what you
have is a dereference to some random vec2 value somewhere. In this
case, we require a cast before the ptr_as_array and use the ptr_stride
field in the cast to provide a stride for the ptr_as_array derefs.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
Instead of just storing the decorations in the vtn_type, propagate them
all the way through to the glsl_type. For array strides, this means we
need to handle them earlier so we break array stride handling into it's
own function and explicitly call it for both pointer and array types.
Due to type deduplication in the SPIR-V, we may have explicit layout
decorations on all sorts of types that don't actually want them. In
order to prevent these leaking into unfortunate places in NIR, we
explicitly strip them off before creating NIR variables and when casting
pointers to non-external memory.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
SPIR-V allows for matrix and array types to be decorated with explicit
byte stride decorations and matrix types to be decorated row- or
column-major. This commit adds support to glsl_type to encode this
information. Because this doesn't work nicely with std430 and std140
alignments, we add asserts to ensure that we don't use any of the std430
or std140 layout functions with explicitly laid out types.
In SPIR-V, the layout information for matrices is applied to the parent
struct member instead of to the matrix type itself. However, this is
gets rather clumsy when you're walking derefs trying to compute offsets
because, the moment you hit a matrix, you have to crawl back the deref
chain and find the struct. Instead, we take the same path here as we've
taken in spirv_to_nir and put the decorations on the matrix type itself.
This also subtly adds support for strided vector types. These don't
come up in SPIR-V directly but you can get one as the result of taking a
column from a row-major matrix or a row from a column-major matrix.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Previously, NIR had a single nir_var_uniform mode used for atomic
counters, UBOs, samplers, images, and normal uniforms. This commit
splits this into nir_var_uniform and nir_var_ubo where nir_var_uniform
is still a bit of a catch-all but the nir_var_ubo is specific to UBOs.
While we're at it, we also rename shader_storage to ssbo to follow the
convention.
We need this so that we can distinguish between normal uniforms and UBO
access at the deref level without going all the way back variable and
seeing if it has an interface type.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
We already had code in link_as_ssa to handle bit sizes; we just need to
use it. While we're at it we clean up link_as_ssa a bit and add an
explicit bit_size parameter in preparation for a day when we have derefs
that aren't 32 bit.
Cc: mesa-stable@lists.freedesktop.org
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
This simplifies our deref handling by emitting the actual NIR deref
instructions on-the-fly instead of of building up a deref chain and then
emitting them at the last moment. In order for this to work with the
parts of the compiler that assume they can chase deref chains, we have
to run nir_rematerialize_derefs_in_use_blocks_impl to put the derefs
back in the right places. Otherwise, in cases such as loop continues
where the SPIR-V blocks are not in the same order as the NIR blocks, we
may end up with a deref chain with a parent that does not dominate it's
child and nir_repair_ssa_impl will insert phis in the deref chain.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
The SPIR-V spec was recently updated to clarify that array indices are
treated as signed integers.
Cc: mesa-stable@lists.freedesktop.org
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
This crops up both in the actual SPIR-V VectorInsert/Extract opcodes as
well as various places where we deal with vector derefs.
Cc: mesa-stable@lists.freedesktop.org
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
This also changes spirv_to_nir and glsl_to_nir to set them. The one
place that doesn't set them is shared memory access lowering in
nir_lower_io. That will have to be updated before any consumers of it
can effectively use these new alignments.
Reviewed-by: Samuel Iglesias Gonsálvez <siglesias@igalia.com>
Acked-by: Karol Herbst <kherbst@redhat.com>
The pattern of adding or multiplying an integer by an immediate is
fairly common especially in deref chain handling. This adds a helper
for it and uses it a few places. The advantage to the helper is that
it automatically handles bit sizes for you.
Reviewed-by: Samuel Iglesias Gonsálvez <siglesias@igalia.com>
Reviewed-by: Karol Herbst <kherbst@redhat.com>
This extension adds two new decorations which carry meaning only for
HLSL shaders. They are expected to be handled by higher level layers
and can be ignored by implementations. However, it does save the client
a bit of work if the implementation safely ignores them instead of the
client having to strip them out of the SPIR-V in order for it to be
valid.
Reviewed-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
v2: - change how the access qualifiers are accumulated
v3: - duplicate members in struct_member_decoration_cb()
- handle access qualifiers on variables
- remove access qualifiers handling in _vtn_variable_load_store()
- fix setting access qualifiers on type->array_element
Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net
Otherwise, they are removed during NIR linking or in some
lowering passes.
Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
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>
GLSL has gl_VertexID which is supposed to be non-zero-based.
SPIR-V has both VertexIndex and VertexId builtins whose meanings are
defined by the APIs.
Vulkan defines VertexIndex as being non-zero-based. In Vulkan VertexId
and InstanceId have no meaning and are pretty much just reserved for
OpenGL at this point.
GL_ARB_spirv removes VertexIndex and defines VertexId to be the same
as gl_VertexId (which is also non-zero-based).
Previously in Mesa it was treating VertexIndex as non-zero-based and
VertexId as zero-based, so it was breaking for GL. This behaviour was
apparently based on Khronos bug 14255. However that bug doesn’t seem
to have made a final decision for VertexId.
Assuming there really is no other definition for VertexId for Vulkan
it seems better to just make them both have the same value.
v2: update comment and commit descriptions, based on Jason Ekstrand
explanation of the meaning/rationale behind all those builtins
(Jason)
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
From SPIR-V 1.0 spec, section 3.20, "Decoration":
"Stream
Apply to an object or a member of a structure type. Indicates the
stream number to put an output on."
Note the "or", so that means that it is allowed for both a full struct
or a membef or a struct (although the wording is not really ideal, and
somewhat error-prone, imho).
We found this with some Geometry Streams tests for ARB_gl_spirv, where
the full gl_PerVertex is assigned Stream 0 (default value on OpenGL
for gl_PerVertex).
So this commit allows structs to have this Decoration, and sets the
stream at the nir variable if needed.
Signed-off-by: Neil Roberts <nroberts@igalia.com>
Signed-off-by: Alejandro Piñeiro <apinheiro@igalia.com>
v2: squash two Decoration Stream patches (Jason)
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
These set the new explicit XFB members on nir_variable.
This is needed to support ARB_gl_spirv, as Vulkan doesn't support
transform feedback.
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
Fixes warning:
../../src/compiler/spirv/vtn_variables.c: In function ‘var_decoration_cb’:
../../src/compiler/spirv/vtn_variables.c:1400:12: warning: ‘is_vertex_input’ may be used uninitialized in this function [-Wmaybe-uninitialized]
bool is_vertex_input;
^~~~~~~~~~~~~~~
The code used to set is_vertex_input in all possible codepaths, but
after 23edc5b1ef "spirv: translate default-block uniforms" the
compiler isn't sure all codepaths will initialize the variable.
Reviewed-by: Anuj Phogat <anuj.phogat@gmail.com>
This is convenient when dealing with atomic counter uniforms. The
alternative would be doing that at vtn_handle_atomics.
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
When constructing NIR if we have a SPIR-V uint variable and the
storage class is SpvStorageClassAtomicCounter, we store as NIR's
glsl_type an atomic_uint to reflect the fact that the variable is an
atomic counter.
However, we were tweaking the type only for atomic_uint scalars, we
have to do it as well for atomic_uint arrays and atomic_uint arrays of
arrays of any depth.
Signed-off-by: Antia Puentes <apuentes@igalia.com>
Signed-off-by: Alejandro Piñeiro <apinheiro@igalia.com>
v2: update after deref patches got pushed (Alejandro Piñeiro)
v3: simplify repair_atomic_type (suggested by Timothy Arceri, included
on the patch by Alejandro)
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
GLSL types differentiates uint from atomic uint. On SPIR-V the type is
uint, and the variable has a specific storage class. So we need to
tweak the type based on the storage class.
Ideally we would like to get the proper type at vtn_handle_type, but
we don't have the storage class at that moment.
We tweak only the nir type, as is the one that really requires it.
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
Also initialize it on var_decoration_cb
This is equivalent to nir_variable.offset, used to store the location
an atomic counter is stored at.
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
This commit completely reworks function calls in NIR. Instead of having
a set of variables for the parameters and return value, nir_call_instr
now has simply has a number of sources which get mapped to load_param
intrinsics inside the functions. It's up to the client API to build an
ABI on top of that. In SPIR-V, out parameters are handled by passing
the result of a deref through as an SSA value and storing to it.
This virtue of this approach can be seen by how much it allows us to
delete from core NIR. In particular, nir_inline_functions gets halved
and goes from a fairly difficult pass to understand in detail to almost
trivial. It also simplifies spirv_to_nir somewhat because NIR functions
never were a good fit for SPIR-V.
Unfortunately, there is no good way to do this without a mega-commit.
Core NIR and SPIR-V have to be changed at the same time. This also
requires changes to anv and radv because nir_inline_functions couldn't
handle deref instructions before this change and can't work without them
after this change.
Acked-by: Rob Clark <robdclark@gmail.com>
Acked-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
Acked-by: Dave Airlie <airlied@redhat.com>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Now that pointers can be derefs and derefs just produce SSA values, we
can convert any pointer to/from SSA.
Acked-by: Rob Clark <robdclark@gmail.com>
Acked-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
Acked-by: Dave Airlie <airlied@redhat.com>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Previously, pointers fell into two categories: index/offset for UBOs,
SSBOs, etc. and var + access chain for logical pointers. This commit
adds another logical pointer mode that's deref + access chain.
It's tempting to think that we can just replace variable-based pointers
with deref-based or at least replace the access chain with a deref
chain. Unfortunately, there are a few sticky bits that prevent this:
1) We can't return deref-based pointers from OpVariable because those
opcodes may come outside of a function so there's no place to emit
the deref instructions.
2) We can't always use variable-based pointers because we may not
always know the variable. (We do now, but he upcoming function
rework will take that option away.)
3) We also can't replace the access chain struct with a deref. Due to
the re-ordering we do in order to handle loop continues, the derefs
we would emit as part of OpAccessChain may not dominate their uses.
We normally fix this up with nir_repair_ssa but that generates phi
nodes which we don't want in the middle of our deref chains.
All in all, we have no real better option than to support partial access
chains while also re-emitting the deref instructions on the spot.
Acked-by: Rob Clark <robdclark@gmail.com>
Acked-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
Acked-by: Dave Airlie <airlied@redhat.com>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Now that push constants are using on-the-fly offsets, we no longer need
to handle access chains in vtn_pointer_to_offset.
Acked-by: Rob Clark <robdclark@gmail.com>
Acked-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
Acked-by: Dave Airlie <airlied@redhat.com>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Push constants have been a weird edge-case for a while in that they have
explitic offsets but we've been internally building access chains for
them. This mostly works but it means that passing pointers to push
constants through as function arguments is broken. The easy thing to do
for now is to just treat them like UBOs or SSBOs only without a block
index. This does loose a bit of information since we no longer have an
accurate access range and any indirect access will look like it could
read the whole block. Unfortunately, there's not much we can do about
that. Once NIR derefs get a bit more powerful, we can plumb these
through as derefs and be able to reason about them again.
Acked-by: Rob Clark <robdclark@gmail.com>
Acked-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
Acked-by: Dave Airlie <airlied@redhat.com>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Before, we were doing structure splitting in spirv_to_nir.
Unfortunately, this doesn't really work when you think about passing
struct pointers into functions. Doing it later in NIR is a much better
plan.
Acked-by: Rob Clark <robdclark@gmail.com>
Acked-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
Acked-by: Dave Airlie <airlied@redhat.com>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Acked-by: Rob Clark <robdclark@gmail.com>
Acked-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
Acked-by: Dave Airlie <airlied@redhat.com>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
The only thing still using old-school drefs are function calls.
Acked-by: Rob Clark <robdclark@gmail.com>
Acked-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
Acked-by: Dave Airlie <airlied@redhat.com>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
When SpvDecorationBinding is encountered in the SPIR-V source it now
sets explicit_binding on the nir_variable. This will be used to
determine whether to initialise sampler and image uniforms with the
binding value.
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
vtn_variable_mode_image and _sampler are instead replaced with
vtn_variable_mode_uniform which encompasses both of them. In the few
places where it was neccessary to distinguish between the two, the
GLSL type of the pointer is used instead.
The main reason to do this is that on OpenGL it is permitted to put
images and samplers into structs and declare a uniform with them. That
means that variables can now have a mix of uniform, sampler and image
modes so picking a single one of those modes for a variable no longer
makes sense.
This fixes OpLoad on a sampler within a struct which was previously
using the variable mode to determine whether it was a sampler or not.
The type of the variable is a struct so it was not being considered to
be uniform mode even though the member being loaded should be sampler
mode.
The previous code appeared to be using var->interface_type as a place
to store the type of the variable without the enclosing array for
images and samplers. I guess this worked because opaque types can not
appear in interfaces so the interface_type is sort of unused. This
patch removes the overloading of var->interface_type and any places
that needed the type without the array can now just deduce it from
var->type.
v2: squash in this patch the changes to anv/nir (Timothy)
Signed-off-by: Eduardo Lima <elima@igalia.com>
Signed-off-by: Neil Roberts <nroberts@igalia.com
Signed-off-by: Alejandro Piñeiro <apinheiro@igalia.com>
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
They are supported by SPIR-V for ARB_gl_spirv.
v2 (changes on top of Nicolai's original patch):
* Handle UniformConstant storage class for uniforms other than
samplers and images. (Eduardo Lima)
* Handle location decoration also for samplers and images. (Eduardo
Lima)
* Rebase update (spirv_to_nir options added, logging changes, and
others) (Alejandro Piñeiro)
Signed-off-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
Signed-off-by: Eduardo Lima <elima@igalia.com>
Signed-off-by: Alejandro Piñeiro <apinheiro@igalia.com>
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
v2:
An attempt to support SpvExecutionModeStencilRefReplacingEXT's behavior
also follows, with the interpretation to said mode being we prevent
writes to the built-in FragStencilRefEXT variable when the execution
mode isn't set.
v3:
A more cautious reading of 1db44252d0 led
me to a missing change that would stop (what I later discovered were)
GPU hangs on the CTS test written to exercise this.
v4:
Turn FragStencilRefEXT decoration usage without StencilRefReplacingEXT
mode into a warning, instead of trying to make the variable read-only.
If we are to follow the originating extension on GL, the built-in
variable in question should never be readable anyway.
v5/v6: rebases.
v7:
Fix check for gen9 lost in rebase. (Ilia)
Reduce the scope of the bool used to track whether
SpvExecutionModeStencilRefReplacingEXT was used. Was in shader_info,
moved to vtn_builder. (Jason)
v8:
Assert for fragment shader handling StencilRefReplacingEXT execution
mode. (Caio)
Remove warning logic, since an entry point might not have
StencilRefReplacingEXT execution mode, but the global output variable
might still exist for another entry point in the module. (Jason)
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
This behaviour was changed in 1e5b09f42f. The commit message
for that says it is just a “tidy up” so my assumption is that the
behaviour change was a mistake. It’s a little hard to decipher looking
at the diff, but the previous code before that patch was:
if (builtin == SpvBuiltInFragCoord || builtin == SpvBuiltInSamplePosition)
nir_var->data.origin_upper_left = b->origin_upper_left;
if (builtin == SpvBuiltInFragCoord)
nir_var->data.pixel_center_integer = b->pixel_center_integer;
After the patch the code was:
case SpvBuiltInSamplePosition:
nir_var->data.origin_upper_left = b->origin_upper_left;
/* fallthrough */
case SpvBuiltInFragCoord:
nir_var->data.pixel_center_integer = b->pixel_center_integer;
break;
Before the patch origin_upper_left affected both builtins and
pixel_center_integer only affected FragCoord. After the patch
origin_upper_left only affects SamplePosition and pixel_center_integer
affects both variables.
This patch tries to restore the previous behaviour by changing the
code to:
case SpvBuiltInFragCoord:
nir_var->data.pixel_center_integer = b->pixel_center_integer;
/* fallthrough */
case SpvBuiltInSamplePosition:
nir_var->data.origin_upper_left = b->origin_upper_left;
break;
This change will be important for ARB_gl_spirv which is meant to
support OriginLowerLeft.
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Anuj Phogat <anuj.phogat@gmail.com>
Fixes: 1e5b09f42f "spirv: Tidy some repeated if checks..."
The base vertex in Vulkan is different from GL in that for non-indexed
primitives the value is taken from the firstVertex parameter instead
of being set to zero. This coincides with the new SYSTEM_VALUE_FIRST_VERTEX
instead of BASE_VERTEX.
v2 (idr): Add comment describing why SYSTEM_VALUE_FIRST_VERTEX is used
for SpvBuiltInBaseVertex. Suggested by Jason.
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com> [v1]
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
OpenCL kernels also have int8/uint8.
v2: remove changes in nir_search as Jason posted a patch for that
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Signed-off-by: Rob Clark <robdclark@gmail.com>
Signed-off-by: Karol Herbst <kherbst@redhat.com>
This capability allows gl_ViewportIndex and gl_Layer to also be used
as outputs in Vertex and Tesselation shaders.
v2: Make conditional to the capability, add gl_Layer, add tesselation
shaders. (Iago)
v3: Don't export to tesselation control shader.
v4: Add Reviewd-by tag.
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
The introduction of 16-bit types with VK_KHR_16bit_storages implies that
push constant offsets could be multiple of 2-bytes. Some assertions are
updated so offsets should be just multiple of size of the base type but
in some cases we can not assume it as doubles aren't aligned to 8 bytes
in some cases.
For 16-bit types, the push constant offset takes into account the
internal offset in the 32-bit uniform bucket adding 2-bytes when we access
not 32-bit aligned elements. In all 32-bit aligned cases it just becomes 0.
v2: Assert offsets to be aligned to the dest type size. (Jason Ekstrand)
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Range in 16-bit push constants load was being calculated
wrongly using 4-bytes per element instead of 2-bytes as it
should be.
v2: Use glsl_get_bit_size instead of if statement
(Jason Ekstrand)
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
The SPIR-V parser splits in/out struct variables and creates
a separate variable for each first-level member of the struct.
When the struct variable has an initializer this means that we also
need to split the initializer.
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Technically, the GLSLang bug related to this can also affect SSBO writes
where the bool -> uint conversion is missing. However, the only known
shipping application with an old enough version of GLSLang to cause
issues with this is the new DOOM game so we keep the workaround as small
as possible.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104424
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Previously, we were storing a pointer to the vtn_value because we use it
to look up decorations when we create input/output variables. This
works, but it also may be useful to have the id itself so we may as well
store that instead.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Now that higher levels are enforcing decoration sanity, we don't need
the vtn_asserts here. This function *should* be safe but we still want
a few well-placed regular asserts in case something goes awry.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Since we switched over to lowering SLM access directly in SPIR-V -> NIR,
we no longer have vtn_variables for SLM. It's all safe as with UBOs and
SSBOs but we need to let it through in the assert.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104213
Fixes: 8761a04d0d
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
There is no chain, so checking the length ends with a SEGFAULT.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103579
Cc: <mesa-stable@lists.freedesktop.org>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
v2: Added more missing implementations of 16-bit types. (Jason Ekstrand)
v3: Store values in values[0].u16[i] (Jason Ekstrand)
Include switches based on bitsize for 16-bit types
(Chema Casanova)
v4: Coding style fixes (Jason Ekstrand)
Use vtn_u64_literal and u64[0] at 64-bit SpvOpConstant (Jason Ekstrand)
Signed-off-by: Jose Maria Casanova Crespo <jmcasanova@igalia.com>
Signed-off-by: Eduardo Lima <elima@igalia.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
The SPIR-V spec is a bit underspecified when it comes to exactly how
you're allowed to use OpPtrAccessChain and what it means in certain edge
cases. In particular, what if the base pointer of the OpPtrAccessChain
points to the base struct of an SSBO instead of an element in that SSBO.
The original variable pointers implementation in mesa assumed that you
weren't allowed to do an OpPtrAccessChain that adjusted the block index
and asserted such. However, there are some CTS tests that do this and,
if the CTS does it, someone will do it in the wild so we should probably
handle it. With this commit, we significantly reduce our assumptions
and should be able to handle more-or-less anything.
The one assumption we still make for correctness is that if we see an
OpPtrAccessChain on a pointer to a struct decorated block that the block
index should be adjusted. In theory, someone could try to put an array
stride on such a pointer and try to make the SSBO an implicit array of
the base struct and we would not give them what they want. That said,
any index other than 0 would count as an out-of-bounds access which is
invalid.
Reviewed-by: Kristian H. Kristensen <hoegsberg@google.com>
Before, we always left workgroup variables as shared nir_variables and
let the driver call nir_lower_io. This adds an option to do the
lowering directly in spirv_to_nir. To do this, we implicitly assign the
variables a std430 layout and then treat them like a UBO or SSBO and
immediately lower all the way to an offset.
As a side-effect, the spirv_to_nir pass now handles variable pointers
for workgroup variables.
Reviewed-by: Kristian H. Kristensen <hoegsberg@google.com>
Up until now, all pointers have been ivec2s. We're about to add support
for pointers to workgroup storage and those are going to be uints.
Reviewed-by: Kristian H. Kristensen <hoegsberg@google.com>
There is no good reason why we should have the same logic repeated in
get_vulkan_resource_index and vtn_ssa_offset_pointer_dereference. If
we're a bit more careful about how we do things, we can just use the one
function and get rid of the other entirely. This also makes the push
constant special case a lot more clear.
Reviewed-by: Kristian H. Kristensen <hoegsberg@google.com>
This commit moves them both into vtn_variables.c towards the top, makes
them take a vtn_builder, and replaces a hand-rolled instance of
is_external_block with a function call.
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Kristian H. Kristensen <hoegsberg@google.com>
This makes us key off of !offset instead of !block_index. It also puts
the guts inside a switch statement so that we can handle more than just
UBOs and SSBOs.
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Kristian H. Kristensen <hoegsberg@google.com>
This parallels what we do for vtn_block_load except that we don't yet
support anything except SSBO loads through this path.
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Kristian H. Kristensen <hoegsberg@google.com>
This is equivalent and means we don't have resource index code scattered
about.
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Kristian H. Kristensen <hoegsberg@google.com>
We have a nir_builder and it has an impl field.
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Kristian H. Kristensen <hoegsberg@google.com>
Jason Ekstrand
Caio Marcelo de Oliveira Filho
Arcady Goldmints-Orlov
Samuel Pitoiset
Karol Herbst
Marek Olšák
Antia Puentes
Lionel Landwerlin
Connor Abbott
Alejandro Piñeiro
Dave Airlie
Bas Nieuwenhuizen
Jason Ekstrand
Timothy Arceri
Kenneth Graunke
Neil Roberts
Rob Clark
Nicolai Hähnle
Gustavo Lima Chaves
Jose Maria Casanova Crespo
Iago Toral Quiroga
Eduardo Lima Mitev