[Mesa-dev] [PATCH v2 06/25] spirv: fix SpvOpSpecConstantOp with SpvOpVectorShuffle working with double-based vecs

[Samuel Iglesias Gonsálvez]

On Mon, 2017-01-02 at 10:10 -0800, Jason Ekstrand wrote:
> 
> 
> On Fri, Dec 16, 2016 at 6:48 AM, Juan A. Suarez Romero <jasuarez at igal
> ia.com> wrote:
> > From: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
> > 
> > We need to pick two 32-bit values per component to perform the
> > right shuffle operation.
> > 
> > Signed-off-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
> > ---
> >  src/compiler/spirv/spirv_to_nir.c | 25 +++++++++++++++++++++----
> >  1 file changed, 21 insertions(+), 4 deletions(-)
> > 
> > diff --git a/src/compiler/spirv/spirv_to_nir.c
> > b/src/compiler/spirv/spirv_to_nir.c
> > index 5303a94..5126dc9 100644
> > --- a/src/compiler/spirv/spirv_to_nir.c
> > +++ b/src/compiler/spirv/spirv_to_nir.c
> > @@ -1073,18 +1073,35 @@ vtn_handle_constant(struct vtn_builder *b,
> > SpvOp opcode,
> >           unsigned len0 = glsl_get_vector_elements(v0->const_type);
> >           unsigned len1 = glsl_get_vector_elements(v1->const_type);
> > 
> > -         uint32_t u[8];
> > +         if (glsl_get_bit_size(v0->const_type) == 64)
> > +            len0 *= 2;
> > +         if (glsl_get_bit_size(v1->const_type) == 64)
> > +            len1 *= 2;
> > +
> > +         /* Allocate space for two dvec4s */
> > +         uint32_t u[16];
> > +         assert(len0 + len1 < 16);
> >           for (unsigned i = 0; i < len0; i++)
> >              u[i] = v0->constant->values[0].u32[i];
> >           for (unsigned i = 0; i < len1; i++)
> >              u[len0 + i] = v1->constant->values[0].u32[i];
> > 
> > -         for (unsigned i = 0; i < count - 6; i++) {
> > +         unsigned bit_size = glsl_get_bit_size(val->const_type);
> 
> It wouldn't hurt to assert that all of the values have the same bit
> size.  In fact, they're all required to have the same base type and
> bit width.  The only thing allowed to vary between the three is the
> number of components.
> 

OK, then I will add that assert.

> In light of that, does it make sense to do as 16 uint32_t's or would
> it work better just to assert that all the bit sizes are the same and
> switch on bit size once?
>  

I am going to simplify this code by taking into account the bit size.
My idea is to have two arrays: one with 8 uint32_t's and another with 8
uint64_t's... and fill-and-use one or the other depending on the
bit_size.

Sam

> > +         for (unsigned i = 0, j = 0; i < count - 6; i++, j++) {
> >              uint32_t comp = w[i + 6];
> > +            /* In case of doubles, we need to pick two 32-bit
> > values,
> > +             * then we duplicate the component to pick the right
> > values.
> > +             */
> > +            if (bit_size == 64)
> > +               comp *= 2;
> >              if (comp == (uint32_t)-1) {
> > -               val->constant->values[0].u32[i] = 0xdeadbeef;
> > +               val->constant->values[0].u32[j] = 0xdeadbeef;
> > +               if (bit_size == 64)
> > +                  val->constant->values[0].u32[++j] = 0xdeadbeef;
> >              } else {
> > -               val->constant->values[0].u32[i] = u[comp];
> > +               val->constant->values[0].u32[j] = u[comp];
> > +               if (bit_size == 64)
> > +                  val->constant->values[0].u32[++j] = u[comp + 1];
> >              }
> >           }
> >           break;
> > --
> > 2.9.3
> > 
> > _______________________________________________
> > 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170103/cc34b054/attachment.sig>