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

2019-01-23 Thread Jason Ekstrand 
On Wed, Jan 23, 2019 at 7:33 AM Sergii Romantsov 
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 
> 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  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 
>>>
>>> On 15/1/19 12:08, Sergii Romantsov wrote:
>>> > During conversion type-length was lost due to math.
>>> >
>>> > CC: Jason Ekstrand 
>>> > 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 
>>> > ---
>>> >   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@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>>
>>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2019-01-23 Thread Sergii Romantsov 
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

So do we still need to use 4?

On Thu, Jan 17, 2019 at 7:13 PM Jason Ekstrand  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  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 
>>
>> On 15/1/19 12:08, Sergii Romantsov wrote:
>> > During conversion type-length was lost due to math.
>> >
>> > CC: Jason Ekstrand 
>> > 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 
>> > ---
>> >   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@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2019-01-17 Thread Jason Ekstrand 
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  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 
>
> On 15/1/19 12:08, Sergii Romantsov wrote:
> > During conversion type-length was lost due to math.
> >
> > CC: Jason Ekstrand 
> > 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 
> > ---
> >   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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2019-01-17 Thread apinheiro 
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 

On 15/1/19 12:08, Sergii Romantsov wrote:

During conversion type-length was lost due to math.

CC: Jason Ekstrand 
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 
---
  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;
*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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2019-01-15 Thread apinheiro 

Just tested it with the ARB_gl_spirv tests where I found this:

Tested-by: Alejandro Piñeiro 


On 15/1/19 12:08, Sergii Romantsov wrote:

During conversion type-length was lost due to math.

CC: Jason Ekstrand 
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 
---
  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;
*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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2019-01-15 Thread Sergii Romantsov 
During conversion type-length was lost due to math.

CC: Jason Ekstrand 
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 
---
 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;
   *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;
}
-- 
2.7.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev