<div dir="ltr"><div dir="ltr"><div>Hello, Jason. Sorry for belated answer.</div><div>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.").</div><div><br></div><div>Additionally with length of 4 CI gives 3 failed tests: <a href="https://mesa-ci.01.org/global_logic/builds/56/group/63a9f0ea7bb98050796b649e85481845">https://mesa-ci.01.org/global_logic/builds/56/group/63a9f0ea7bb98050796b649e85481845</a>.</div><div>With length of 1 it passes: <a href="https://mesa-ci.01.org/global_logic/builds/57/group/63a9f0ea7bb98050796b649e85481845">https://mesa-ci.01.org/global_logic/builds/57/group/63a9f0ea7bb98050796b649e85481845</a></div><div><br></div><div>So do we still need to use 4?</div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Jan 17, 2019 at 7:13 PM Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr">I think this is probably mostly ok.  There's still some question about exactly what gets stored there and who is storing it.<br></div><br><div class="gmail_quote"><div dir="ltr">On Thu, Jan 17, 2019 at 4:34 AM apinheiro <<a href="mailto:apinheiro@igalia.com" target="_blank">apinheiro@igalia.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">I was waiting Jason to chime in, but just in case he is too busy I took <br>
a look in detail to the patch. It LGTM, so assuming it passes intel CI:<br>
<br>
Reviewed-by: Alejandro Piñeiro <<a href="mailto:apinheiro@igalia.com" target="_blank">apinheiro@igalia.com</a>><br>
<br>
On 15/1/19 12:08, Sergii Romantsov wrote:<br>
> During conversion type-length was lost due to math.<br>
><br>
> CC: Jason Ekstrand <<a href="mailto:jason.ekstrand@intel.com" target="_blank">jason.ekstrand@intel.com</a>><br>
> Fixes: 44227453ec03 (nir: Switch to using 1-bit Booleans for almost everything)<br>
> Bugzilla: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=109353" rel="noreferrer" target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=109353</a><br>
> Signed-off-by: Sergii Romantsov <<a href="mailto:sergii.romantsov@globallogic.com" target="_blank">sergii.romantsov@globallogic.com</a>><br>
> ---<br>
>   src/compiler/spirv/spirv_to_nir.c | 9 ++++++---<br>
>   1 file changed, 6 insertions(+), 3 deletions(-)<br>
><br>
> diff --git a/src/compiler/spirv/spirv_to_nir.c b/src/compiler/spirv/spirv_to_nir.c<br>
> index e3dc619..faad771 100644<br>
> --- a/src/compiler/spirv/spirv_to_nir.c<br>
> +++ b/src/compiler/spirv/spirv_to_nir.c<br>
> @@ -1042,14 +1042,16 @@ vtn_type_layout_std430(struct vtn_builder *b, struct vtn_type *type,<br>
>   {<br>
>      switch (type->base_type) {<br>
>      case vtn_base_type_scalar: {<br>
> -      uint32_t comp_size = glsl_get_bit_size(type->type) / 8;<br>
> +      uint32_t comp_size = glsl_type_is_boolean(type->type)<br>
> +         ? 1 : glsl_get_bit_size(type->type) / 8;<br></blockquote><div><br></div><div>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.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
>         *size_out = comp_size;<br>
>         *align_out = comp_size;<br>
>         return type;<br>
>      }<br>
>   <br>
>      case vtn_base_type_vector: {<br>
> -      uint32_t comp_size = glsl_get_bit_size(type->type) / 8;<br>
> +      uint32_t comp_size = glsl_type_is_boolean(type->type)<br>
> +         ? 1 : glsl_get_bit_size(type->type) / 8;<br>
>         unsigned align_comps = type->length == 3 ? 4 : type->length;<br>
>         *size_out = comp_size * type->length,<br>
>         *align_out = comp_size * align_comps;<br>
> @@ -1168,7 +1170,8 @@ vtn_handle_type(struct vtn_builder *b, SpvOp opcode,<br>
>         val->type->base_type = vtn_base_type_vector;<br>
>         val->type->type = glsl_vector_type(glsl_get_base_type(base->type), elems);<br>
>         val->type->length = elems;<br>
> -      val->type->stride = glsl_get_bit_size(base->type) / 8;<br>
> +      val->type->stride = glsl_type_is_boolean(val->type->type)<br>
> +         ? 1 : glsl_get_bit_size(base->type) / 8;<br>
>         val->type->array_element = base;<br>
>         break;<br>
>      }<br>
_______________________________________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br>
</blockquote></div></div>
</blockquote></div>