[Mesa-dev] [PATCH 1/2] spirv: Add support for SpvCapabilityStorageImageWriteWithoutFormat
Iago Toral
itoral at igalia.com
Tue Feb 14 12:08:26 UTC 2017
On Tue, 2017-02-14 at 11:42 +0000, Alex Smith wrote:
> Hi Iago,
>
> On 14 February 2017 at 11:19, Iago Toral <itoral at igalia.com> wrote:
> > On Tue, 2017-02-14 at 09:46 +0000, Alex Smith wrote:
> > > On 14 February 2017 at 08:45, Iago Toral <itoral at igalia.com>
> > wrote:
> > > > On Mon, 2017-02-13 at 16:29 +0000, Lionel Landwerlin wrote:
> > > > > Run this by our CI earlier today and got a few failures :
> > > > >
> > > > > dEQP-VK.image.load_store.buffer.r8g8b8a8_snorm
> > > > > dEQP-VK.image.load_store.buffer.r8g8b8a8_unorm
> > > > > dEQP-
> > VK.image.format_reinterpret.buffer.r32_uint_r8g8b8a8_snorm
> > > > > dEQP-
> > > > VK.image.format_reinterpret.buffer.r8g8b8a8_uint_r8g8b8a8_unorm
> > > > > dEQP-
> > > > VK.image.format_reinterpret.buffer.r8g8b8a8_sint_r8g8b8a8_snorm
> > > > > dEQP-
> > > > VK.image.format_reinterpret.buffer.r8g8b8a8_sint_r8g8b8a8_unorm
> > > > > dEQP-
> > VK.image.format_reinterpret.buffer.r32_sint_r8g8b8a8_unorm
> > > > > dEQP-
> > VK.image.format_reinterpret.buffer.r32_sint_r8g8b8a8_snorm
> > > > > dEQP-
> > VK.image.format_reinterpret.buffer.r32_uint_r8g8b8a8_unorm
> > > > > dEQP-
> > > >
> > VK.image.format_reinterpret.buffer.r8g8b8a8_snorm_r8g8b8a8_unorm
> > > > > dEQP-VK.image.store.buffer.r8g8b8a8_unorm
> > > > > dEQP-VK.image.store.buffer.r8g8b8a8_snorm
> > > > >
> > > > > I'm not quite sure why, it seems our backend discards format
> > > > layout
> > > > > qualifiers when we have writeonly set.
> > > >
> > > > For what is worth, I see a lot more regressions from
> > > > image.load_store
> > > > tests with this patch. Some of these get fixed with the second
> > > > patch,
> > > > but even then I see all of dEQP-
> > > > VK.image.load_store.*.r32g32b32a32.*
> > > > regressing (with Lionel's comment to patch 2 fixed)
> > > These are all fixed by applying Jason's patch
> > > "anv/apply_pipeline_layout: Set image.write_only to false" [1]
> > before
> > > this one. I've just tested locally and see no failures with both
> > that
> > > patch and this one applied. I'll include that patch when I resend
> > the
> > > series.
> >
> > Right, I can confirm this too. The second patch in the series, even
> > with Lionel's comment fixed, does seem to add regressions even with
> > Jason's patch though.
> Were these failures on buffer-related tests (e.g. dEQP-
> VK.image.load_store.buffer.*)?
>
> If so, I've fixed these in the new version of the patch I submitted a
> few hours ago - buffer views also needed to be changed to handle
> write-only accesses in the same way as image views. With the new
> patch I see no failures on the dEQP-VK.image.* tests.
Yes, I can confirm that your v2 passes all of dEQP-
VK.image.{load_}store.* on my end too. Also, I have done a quick test
with some CTS tests removing the image format for the writeonly images
in the dEQP-VK.image.store.* tests and they all seem to pass (this
needs the patch I sent to the list today), so I think it looks good.
Iago
> Thanks,
> Alex
>
> >
> > Iago
> >
> > > Alex
> > >
> > > [1] https://lists.freedesktop.org/archives/mesa-dev/2017-February
> > /144
> > > 167.html
> > >
> > > >
> > > > Iago
> > > >
> > > > > -
> > > > > Lionel
> > > > >
> > > > > On 13/02/17 16:10, Jason Ekstrand wrote:
> > > > > > Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>
> > > > > >
> > > > > > On Thu, Feb 9, 2017 at 8:06 AM, Alex Smith <asmith at feralint
> > erac
> > > > tive
> > > > > > .com> wrote:
> > > > > > > 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 at feralinteractive.com>
> > > > > > > ---
> > > > > > > src/compiler/spirv/nir_spirv.h | 1 +
> > > > > > > src/compiler/spirv/spirv_to_nir.c | 5 ++++-
> > > > > > > src/compiler/spirv/vtn_variables.c | 5 ++++-
> > > > > > > 3 files changed, 9 insertions(+), 2 deletions(-)
> > > > > > >
> > > > > > > diff --git a/src/compiler/spirv/nir_spirv.h
> > > > > > > b/src/compiler/spirv/nir_spirv.h
> > > > > > > index e0ebc62..e43e9b5 100644
> > > > > > > --- a/src/compiler/spirv/nir_spirv.h
> > > > > > > +++ b/src/compiler/spirv/nir_spirv.h
> > > > > > > @@ -49,6 +49,7 @@ struct nir_spirv_supported_extensions {
> > > > > > > bool image_ms_array;
> > > > > > > bool tessellation;
> > > > > > > bool draw_parameters;
> > > > > > > + bool image_write_without_format;
> > > > > > > };
> > > > > > >
> > > > > > > nir_function *spirv_to_nir(const uint32_t *words, size_t
> > > > > > > word_count,
> > > > > > > diff --git a/src/compiler/spirv/spirv_to_nir.c
> > > > > > > b/src/compiler/spirv/spirv_to_nir.c
> > > > > > > index 416e12a..3f77ddf 100644
> > > > > > > --- a/src/compiler/spirv/spirv_to_nir.c
> > > > > > > +++ b/src/compiler/spirv/spirv_to_nir.c
> > > > > > > @@ -2666,7 +2666,6 @@
> > vtn_handle_preamble_instruction(struct
> > > > > > > vtn_builder *b, SpvOp opcode,
> > > > > > > case SpvCapabilityMinLod:
> > > > > > > case SpvCapabilityTransformFeedback:
> > > > > > > case SpvCapabilityStorageImageReadWithoutFormat:
> > > > > > > - case SpvCapabilityStorageImageWriteWithoutFormat:
> > > > > > > vtn_warn("Unsupported SPIR-V capability: %s",
> > > > > > > spirv_capability_to_string(cap));
> > > > > > > break;
> > > > > > > @@ -2702,6 +2701,10 @@
> > vtn_handle_preamble_instruction(struct
> > > > > > > vtn_builder *b, SpvOp opcode,
> > > > > > > spv_check_supported(draw_parameters, cap);
> > > > > > > break;
> > > > > > >
> > > > > > > + case SpvCapabilityStorageImageWriteWithoutFormat:
> > > > > > > + spv_check_supported(image_write_without_format,
> > > > cap);
> > > > > > > + break;
> > > > > > > +
> > > > > > > default:
> > > > > > > unreachable("Unhandled capability");
> > > > > > > }
> > > > > > > diff --git a/src/compiler/spirv/vtn_variables.c
> > > > > > > b/src/compiler/spirv/vtn_variables.c
> > > > > > > index 098cfb5..d7d882e 100644
> > > > > > > --- a/src/compiler/spirv/vtn_variables.c
> > > > > > > +++ b/src/compiler/spirv/vtn_variables.c
> > > > > > > @@ -1054,8 +1054,12 @@ apply_var_decoration(struct
> > > > vtn_builder
> > > > > > > *b, nir_variable *nir_var,
> > > > > > > assert(nir_var->constant_initializer != NULL);
> > > > > > > nir_var->data.read_only = true;
> > > > > > > break;
> > > > > > > + case SpvDecorationNonReadable:
> > > > > > > + nir_var->data.image.write_only = true;
> > > > > > > + break;
> > > > > > > case SpvDecorationNonWritable:
> > > > > > > nir_var->data.read_only = true;
> > > > > > > + nir_var->data.image.read_only = true;
> > > > > > > break;
> > > > > > > case SpvDecorationComponent:
> > > > > > > nir_var->data.location_frac = dec->literals[0];
> > > > > > > @@ -1107,7 +1111,6 @@ apply_var_decoration(struct
> > vtn_builder
> > > > *b,
> > > > > > > nir_variable *nir_var,
> > > > > > > case SpvDecorationAliased:
> > > > > > > case SpvDecorationVolatile:
> > > > > > > case SpvDecorationCoherent:
> > > > > > > - case SpvDecorationNonReadable:
> > > > > > > case SpvDecorationUniform:
> > > > > > > case SpvDecorationStream:
> > > > > > > case SpvDecorationOffset:
> > > > > > > --
> > > > > > > 2.7.4
> > > > > > >
> > > > > > > _______________________________________________
> > > > > > > mesa-dev mailing list
> > > > > > > mesa-dev at lists.freedesktop.org
> > > > > > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > > > > > >
> > > > > >
> > > > > >
> > > > > > _______________________________________________
> > > > > > mesa-dev mailing list
> > > > > > mesa-dev at lists.freedesktop.org
> > > > > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > > > > _______________________________________________
> > > > > mesa-dev mailing list
> > > > > mesa-dev at lists.freedesktop.org
> > > > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > > >
> >
More information about the mesa-dev
mailing list