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>
Jason Ekstrand
Alex Smith
Samuel Iglesias Gonsálvez
Jason Ekstrand
Matt Turner
Dave Airlie
Lionel Landwerlin
Iago Toral Quiroga
Kenneth Graunke