[Mesa-dev] [PATCH 1/2] spirv: Add support for SpvCapabilityStorageImageWriteWithoutFormat

[Iago Toral]

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)

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 feralinteractive
> > .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