[Mesa-dev] [PATCH v1] nir: Length of boolean vtn_value now is 1

[Jason Ekstrand]

On Wed, Jan 23, 2019 at 7:33 AM Sergii Romantsov <sergii.romantsov at gmail.com>
wrote:

> Hello, Jason. Sorry for belated answer.
> Why length of 1 was selected: primary type SpvOpTypeBool is already using
> length 1 (and description "For Workgroup pointers, this is the size of the
> referenced type.").
>
> Additionally with length of 4 CI gives 3 failed tests:
> https://mesa-ci.01.org/global_logic/builds/56/group/63a9f0ea7bb98050796b649e85481845
> .
> With length of 1 it passes:
> https://mesa-ci.01.org/global_logic/builds/57/group/63a9f0ea7bb98050796b649e85481845
>

I think those failures are spurious.  I've not been able to reproduce
either of them locally.  I'm re-running the CI run and I expect it'll
pass.  If it does, I'll push this with 4.

--Jason


> So do we still need to use 4?
>
> On Thu, Jan 17, 2019 at 7:13 PM Jason Ekstrand <jason at jlekstrand.net>
> wrote:
>
>> I think this is probably mostly ok.  There's still some question about
>> exactly what gets stored there and who is storing it.
>>
>> On Thu, Jan 17, 2019 at 4:34 AM apinheiro <apinheiro at igalia.com> wrote:
>>
>>> I was waiting Jason to chime in, but just in case he is too busy I took
>>> a look in detail to the patch. It LGTM, so assuming it passes intel CI:
>>>
>>> Reviewed-by: Alejandro Piñeiro <apinheiro at igalia.com>
>>>
>>> On 15/1/19 12:08, Sergii Romantsov wrote:
>>> > During conversion type-length was lost due to math.
>>> >
>>> > CC: Jason Ekstrand <jason.ekstrand at intel.com>
>>> > Fixes: 44227453ec03 (nir: Switch to using 1-bit Booleans for almost
>>> everything)
>>> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109353
>>> > Signed-off-by: Sergii Romantsov <sergii.romantsov at globallogic.com>
>>> > ---
>>> >   src/compiler/spirv/spirv_to_nir.c | 9 ++++++---
>>> >   1 file changed, 6 insertions(+), 3 deletions(-)
>>> >
>>> > diff --git a/src/compiler/spirv/spirv_to_nir.c
>>> b/src/compiler/spirv/spirv_to_nir.c
>>> > index e3dc619..faad771 100644
>>> > --- a/src/compiler/spirv/spirv_to_nir.c
>>> > +++ b/src/compiler/spirv/spirv_to_nir.c
>>> > @@ -1042,14 +1042,16 @@ vtn_type_layout_std430(struct vtn_builder *b,
>>> struct vtn_type *type,
>>> >   {
>>> >      switch (type->base_type) {
>>> >      case vtn_base_type_scalar: {
>>> > -      uint32_t comp_size = glsl_get_bit_size(type->type) / 8;
>>> > +      uint32_t comp_size = glsl_type_is_boolean(type->type)
>>> > +         ? 1 : glsl_get_bit_size(type->type) / 8;
>>>
>>
>> Because we don't know in spirv_to_nir what size the back-end is going to
>> use for booleans, we should assume they're 32-bit and use 4 here and
>> below.  Otherwise, we may end up with booleans overlapping other stuff.
>>
>>
>>> >         *size_out = comp_size;
>>> >         *align_out = comp_size;
>>> >         return type;
>>> >      }
>>> >
>>> >      case vtn_base_type_vector: {
>>> > -      uint32_t comp_size = glsl_get_bit_size(type->type) / 8;
>>> > +      uint32_t comp_size = glsl_type_is_boolean(type->type)
>>> > +         ? 1 : glsl_get_bit_size(type->type) / 8;
>>> >         unsigned align_comps = type->length == 3 ? 4 : type->length;
>>> >         *size_out = comp_size * type->length,
>>> >         *align_out = comp_size * align_comps;
>>> > @@ -1168,7 +1170,8 @@ vtn_handle_type(struct vtn_builder *b, SpvOp
>>> opcode,
>>> >         val->type->base_type = vtn_base_type_vector;
>>> >         val->type->type =
>>> glsl_vector_type(glsl_get_base_type(base->type), elems);
>>> >         val->type->length = elems;
>>> > -      val->type->stride = glsl_get_bit_size(base->type) / 8;
>>> > +      val->type->stride = glsl_type_is_boolean(val->type->type)
>>> > +         ? 1 : glsl_get_bit_size(base->type) / 8;
>>> >         val->type->array_element = base;
>>> >         break;
>>> >      }
>>> _______________________________________________
>>> 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/20190123/d411b498/attachment-0001.html>