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

[Jason Ekstrand]

On Thu, Jan 5, 2017 at 12:39 PM, Jason Ekstrand <jason at jlekstrand.net>
wrote:

> On Thu, Jan 5, 2017 at 2:18 AM, Samuel Iglesias Gonsálvez <
> siglesias at igalia.com> wrote:
>
>> We need to pick two 32-bit values per component to perform the right
>> shuffle operation.
>>
>> v2 (Jason):
>> - Add assert to check matching bit sizes (Jason)
>> - Simplify the code to pick components (Jason)
>>
>> Signed-off-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
>> ---
>>  src/compiler/spirv/spirv_to_nir.c | 40 ++++++++++++++++++++++++++++++
>> ---------
>>  1 file changed, 31 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/compiler/spirv/spirv_to_nir.c
>> b/src/compiler/spirv/spirv_to_nir.c
>> index b44b8e823d2..a9c1bef1411 100644
>> --- a/src/compiler/spirv/spirv_to_nir.c
>> +++ b/src/compiler/spirv/spirv_to_nir.c
>> @@ -1076,18 +1076,40 @@ 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];
>> -         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];
>> +         uint32_t u32[8];
>> +         uint64_t u64[8];
>> +         assert(len0 + len1 < 16);
>> +
>> +         unsigned bit_size = glsl_get_bit_size(val->const_type);
>> +         assert(bit_size == glsl_get_bit_size(v0->const_type) &&
>> +                bit_size == glsl_get_bit_size(v1->const_type));
>> +
>> +         for (unsigned i = 0; i < len0; i++) {
>> +            if (bit_size == 64)
>> +               u64[i] = v0->constant->values[0].u64[i];
>> +            else
>> +               u32[i] = v0->constant->values[0].u32[i];
>> +         }
>> +
>> +         for (unsigned i = 0; i < len1; i++) {
>> +            if (bit_size == 64)
>>
>
> Can we please switch on bit_size once?  I think it's probably actually
> easier to read that way.  It's not a complicated blob of code but having
> everything depend on bit_size makes it look more complex than it is.  Also,
> for what it's worth, switching once is more efficient.\
>

With that,

Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>


>
>
>> +               u64[len0 + i] = v1->constant->values[0].u64[i];
>> +            else
>> +               u32[len0 + i] = v1->constant->values[0].u32[i];
>> +         }
>>
>> -         for (unsigned i = 0; i < count - 6; i++) {
>> +         for (unsigned i = 0, j = 0; i < count - 6; i++, j++) {
>>              uint32_t comp = w[i + 6];
>> -            if (comp == (uint32_t)-1) {
>> -               val->constant->values[0].u32[i] = 0xdeadbeef;
>> +            if (bit_size == 64) {
>> +               if (comp == (uint32_t)-1)
>> +                  val->constant->values[0].u64[j] = 0xdeadbeefdeadbeef;
>> +               else
>> +                  val->constant->values[0].u64[j] = u64[comp];
>>              } else {
>> -               val->constant->values[0].u32[i] = u[comp];
>> +               if (comp == (uint32_t)-1)
>> +                  val->constant->values[0].u32[j] = 0xdeadbeef;
>> +               else
>> +                  val->constant->values[0].u32[j] = u32[comp];
>>              }
>>           }
>>           break;
>> --
>> 2.11.0
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170105/6ed0f38e/attachment.html>