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>
We should use the result type of the OpSampledImage opcode, rather than
the type of the underlying image/samplers.
This resolves an issue when using separate images and shadow samplers
with glslang. Example:
layout (...) uniform samplerShadow s0;
layout (...) uniform texture2D res0;
...
float result = textureLod(sampler2DShadow(res0, s0), uv, 0);
For this, for the combined OpSampledImage, the type of the base image
was being used (which does not have the Depth flag set, whereas the
result type does), therefore it was not being recognised as a shadow
sampler. This led to the wrong LLVM intrinsics being emitted by RADV.
Signed-off-by: Alex Smith <asmith@feralinteractive.com>
Cc: "17.2 17.3" <mesa-stable@lists.freedesktop.org>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Currently we support 32-bit indexes/offsets all over the driver, so we
convert them to that bit size.
Fixes dEQP-VK.spirv_assembly.instruction.*.indexing.*
v2: Use u2u32 instead (Jason).
Signed-off-by: Samuel Iglesias Gonsálvez <siglesias@igalia.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
I have no idea how this got missed but it's been missing since forever.
Cc: mesa-stable@lists.freedesktop.org
Reviewed-by: Samuel Iglesias Gonsálvez <siglesias@igalia.com>
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
Quiets a number of uninitialized variable warnings in clang.
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com>
Now that we have proper pointer types, we can be more sensible about the
way we set up function arguments and deal with the two cases of pointer
vs. SSA parameters distinctly.
Reviewed-by: Connor Abbott <cwabbott0@gmail.com>
Now that we have a pointer wrapper class, we can create offsets for UBOs
and SSBOs up-front instead of waiting until we have the full access
chain. For push constants, we still use the old mechanism because it
provides us with some nice range information.
Reviewed-by: Connor Abbott <cwabbott0@gmail.com>
Everyone now calls it with stop_at_matrix = false. Since we're now
always walking all the way to the end of the access chain, the type
returned is just the same as ptr->type;
Reviewed-by: Connor Abbott <cwabbott0@gmail.com>
Instead of handling all of the complexity at the end, we choose to
decorate types a bit more cleverly. When we have a row-major matrix
type, we give it the stride of a single vector and give it's array
element type (which represents a column) the actual matrix stride.
Previously, we were using stop_at_matrix and handling everything from
matrix on down as special cases but now we walk the access chain all the
way to the end and then load. Even though this looks like it may lead
to a significant functional change, it doesn't. The reason why we
needed to do stop_at_matrix before was to handle row-major properly
since the offsets and strides would be all out-of-order. Now that row
major matrix types have the small stride on the matrix and the large
stride on the vector, offsetting to a single column of a row-major
matrix works fine. The load/store code simply picks up on the fact that
the stride isn't the type size and does multiple loads. The generated
code from these methods should be the same.
Reviewed-by: Connor Abbott <cwabbott0@gmail.com>
The vtn_pointer structure provides a bit better abstraction than passing
access chains around directly. For one thing, if the pointer just
points to a variable, we don't need the access chain at all. Also,
pointers know what their dereferenced type is so we can avoid passing
the type in a bunch of places. Finally, pointers can, in theory, be
extended to the case where you don't actually know what variable is
being referenced.
Reviewed-by: Connor Abbott <cwabbott0@gmail.com>
We're about to add a vtn_pointer data structure and this will prevent
some rename churn in the next commit.
Reviewed-by: Connor Abbott <cwabbott0@gmail.com>
We were originally handling them together because I was rather unclear
on the distinction. However, keeping them combined keeps the confusion.
Split them up so that it's more clear from the code how we expect the
two storage classes to be used.
Reviewed-by: Connor Abbott <cwabbott0@gmail.com>
It's closing a "{" at the begining of a switch case.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Connor Abbott <cwabbott0@gmail.com>
Doom shipped with a broken version of GLSLang which handles samplers as
function arguments in a way that isn't spec-compliant. In particular,
it creates a temporary local sampler variable and copies the sampler
into it. While Dave has had a hack patch out for a while that gets it
working, we've never landed it because we've been hoping that a game
update would come out with fixed shaders. Unfortunately, no game update
appears on to be on the horizon and I've found this issue in yet another
application so I think we're stuck working around it. Hopefully, we can
delete this code one day.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99467
Cc: "17.1" <mesa-stable@lists.freedesktop.org>
Tested-by: Grazvydas Ignotas <notasas@gmail.com>
Reviewed-by: Dave Airlie <airlied@redhat.com>
Commit e1af20f18a changed the shader_info
from being embedded into being just a pointer. The idea was that
sharing the shader_info between NIR and GLSL would be easier if it were
a pointer pointing to the same shader_info struct. This, however, has
caused a few problems:
1) There are many things which generate NIR without GLSL. This means
we have to support both NIR shaders which come from GLSL and ones
that don't and need to have an info elsewhere.
2) The solution to (1) raises all sorts of ownership issues which have
to be resolved with ralloc_parent checks.
3) Ever since 00620782c9, we've been
using nir_gather_info to fill out the final shader_info. Thanks to
cloning and the above ownership issues, the nir_shader::info may not
point back to the gl_shader anymore and so we have to do a copy of
the shader_info from NIR back to GLSL anyway.
All of these issues go away if we just embed the shader_info in the
nir_shader. There's a little downside of having to copy it back after
calling nir_gather_info but, as explained above, we have to do that
anyway.
Acked-by: Timothy Arceri <tarceri@itsqueeze.com>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
This adds the spirv->nir conversion for int64 types.
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Allow that capability if the driver indicates that it is supported, and
flag whether images are read-only/write-only in the nir_variable (based
on the NonReadable and NonWritable decorations), which drivers may need
to implement this.
Signed-off-by: Alex Smith <asmith@feralinteractive.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
SPIR-V maps both gl_SampleMask and gl_SampleMaskIn to the same
builtin (SampleMask). The only way to tell which one we are dealing with
is to check if it is an input or an output.
Fixes:
dEQP-VK.pipeline.multisample_shader_builtin.sample_mask.write.*
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Anuj Phogat <anuj.phogat@gmail.com>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Some applications might add location decoration to samplers. Rather
than raising an error it seems it would make more sense to just
discard these decorations.
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Cc: 17.0 <mesa-stable@lists.freedesktop.org>
Once again, SPIR-V is insane... It allows you to place "patch"
decorations on structure members. Presumably, this is so that you can
do something such as
out struct S {
layout(location = 0) patch vec4 thing1;
layout(location = 0) vec4 thing2;
} str;
And have your I/O "nicely" organized. While this is a bit silly, it's
allowed and well-defined so whatever. Where it really gets interesting
is when you have an array of struct. SPIR-V says nothing about not
allowing you to have those qualifiers on the members of a struct that's
inside an array and GLSLang does this. Specifically, if you have
layout(location = 0) out patch struct S {
vec4 thing1;
vec4 thing2;
} str[2];
then GLSLang will place the "patch" decorations on the struct members.
This is ridiculous there is no way that having some of them be patch and
some not would be well-defined given that patch and non-patch outputs
are in effectively different storage classes. This commit moves around
the way we handle the "patch" decoration so that we can detect even the
crazy cases and handle them.
Fixes: dEQP-VK.tessellation.user_defined_io.per_patch_block_array.*
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Geometry and Tessellation stages do handle this as a system value instead.
Fixes:
dEQP-VK.geometry.basic.primitive_id
Reviewed-by: Dave Airlie <ailried@redhat.com>
We need to:
- handle the extra array level for per-vertex varyings
- handle the patch qualifier correctly
- assign varying locations
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Dave Airlie <airlied@redhat.com>
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
We rename it to nir_deref_clone, re-order the sources to match the other
clone functions, and expose nir_deref_var_clone. This past part, in
particular, lets us get rid of quite a few lines since we no longer have
to call nir_copy_deref and wrap it in deref_as_var.
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
All of these are happily set from glsl_to_nir or spirv_to_nir but their
values are never used for anything.
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
This has bothered me for about as long as NIR has been around. Why do we
have two different unions for constants? No good reason other than one of
them is a direct port from GLSL IR.
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Before, we were always treating it as an output which bogus. The only
stage in which this it can be an output is the geometry stage. In all
other stages, it's an input which, in the back-end, we actually want to be
a system value.
Cc: "13.0" <mesa-stable@lists.freedesktop.org>
Reviewed-by: Dave Airlie <airlied@redhat.com>
No change in behavior. ralloc_size is equivalent to rzalloc_size.
That will change though.
Calls not switched to rzalloc_size:
- ralloc_vasprintf
- glsl_type::name allocation (it's filled with snprintf)
- C++ classes where valgrind didn't show uninitialized values
I switched most of non-glsl stuff to rzalloc without checking whether
it's really needed.
Reviewed-by: Edward O'Callaghan <funfunctor@folklore1984.net>
Tested-by: Edmondo Tommasina <edmondo.tommasina@gmail.com>
Reviewed-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
When restoring something from shader cache we won't have and don't
want to create a nir_shader this change detaches the two.
There are other advantages such as being able to reuse the
shader info populated by GLSL IR.
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
This just translates to the correct cull distance slot.
Reviewed-by: Edward O'Callaghan <funfunctor@folklore1984.net>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Previously, we dind't apply variable decorations to the members of a split
structure variable. This doesn't quite work, unfortunately, because things
such as the "flat" qualifier may get applied to an entire structure instead
of propagated to the members. This fixes 9 of the new CTS tests in the
dEQP-VK.glsl.linkage.varying.struct.* group.
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
Cc: "12.0" <mesa-stable@lists.freedesktop.org>
Likewise, rename the enum type to glsl_interp_mode.
Beyond the GLSL front-end, talking about "interpolation modes" seems
more natural than "interpolation qualifiers" - in the IR, we're removed
from how exactly the source language specifies how to interpolate an
input. Also, SPIR-V calls these "decorations" rather than "qualifiers".
Generated by:
$ find . -regextype egrep -regex '.*\.(c|cpp|h)' -type f -exec sed -i \
-e 's/INTERP_QUALIFIER_/INTERP_MODE_/g' \
-e 's/glsl_interp_qualifier/glsl_interp_mode/g' {} \;
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Acked-by: Dave Airlie <airlied@redhat.com>
I have no idea why we were multiplying by 4 before. The offsets we get
from SPIR-V are in bytes and so is nir->num_uniforms so there's no need to
do any adjustment whatsoever.
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
Cc: "12.0" <mesa-stable@lists.freedesktop.org>
SPIR-V treats it as an input but NIR wants the system value. This
shouldn't have been too much of a surprise given that we have to do the
same conversion in the GLSL IR to NIR pass.
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Cc: "12.0" <mesa-stable@lists.freedesktop.org>
glslang frequently throw bogus decorations into shaders. While we are free
to assert-fail, it's a bit nicer to the application to just warn.
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
Cc: "12.0" <mesa-stable@lists.freedesktop.org>
SPIR-V specifies that a bunch of stuff gets applied to types. This means
taht a local variable could get, for instance, an array stride. Just
because it's pointless doesn't mean you'll never see it.
From time to time we have had cases where glslang has added a decoration we
don't handle and it has caused problems. This audit ensures that, for
every decoration, we either handle it or hit an unreachable() with an
accurate description of why we don't have to.
This isn't allowed by Vulkan, but might be useful someday for
SPIR-V in OpenGL (if that ever becomes a thing). It's easy enough
to hook up, and as precedent, we already do so for OriginLowerLeft.
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
It's what all the call-sites once, so gets rid of a bunch of inlined
glsl_get_base_type() at the call-sites.
Signed-off-by: Rob Clark <robclark@freedesktop.org>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
We want to use interface_type, not vtn_var->type. They're normally
equivalent, but for geometry/tessellation per-vertex interface arrays,
we need to unwrap a level.
Otherwise, we tried to iterate a structure members but instead used
an array length. If the array length was longer than the number of
fields in the structure, we'd crash.
Fixes the CreatePipelineGeometryInputBlockPositive layer validation
test.
v2: Just use glsl_without_array() on the vtn_var type
(requested by Jason Ekstrand).
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Chris Forbes <chrisforbes@google.com>
While it does rely on NIR, it's not really part of the NIR core. At the
moment, it still builds as part of libnir but that can be changed later if
desired.
Neil Roberts
Caio Marcelo de Oliveira Filho
Rob Clark
Karol Herbst
Alejandro Piñeiro
Antia Puentes
Jason Ekstrand
Nicolai Hähnle
Gustavo Lima Chaves
Jose Maria Casanova Crespo
Samuel Pitoiset
Iago Toral Quiroga
Bas Nieuwenhuizen
Eduardo Lima Mitev
Alex Smith
Samuel Iglesias Gonsálvez
Jason Ekstrand
Matt Turner
Dave Airlie
Lionel Landwerlin
Kenneth Graunke
Marek Olšák
Timothy Arceri
Rob Clark