Re: [Mesa-dev] [PATCH v3 05/43] nir: Populate conversion opcodes to/from 16-bit types

[Chema Casanova]

El 21/10/17 a las 11:44, Pohjolainen, Topi escribió:
> On Sat, Oct 21, 2017 at 11:22:45AM +0300, Pohjolainen, Topi wrote:
>> On Thu, Oct 12, 2017 at 08:37:54PM +0200, Jose Maria Casanova Crespo wrote:
>>> From: Eduardo Lima Mitev 
>>>
>>> This will include the following NIR ALU opcodes:
>>>  * nir_op_i2i16
>>>  * nir_op_i2f16
>>>  * nir_op_u2u16
>>>  * nir_op_u2f16
>>>  * nir_op_f2i16
>>>  * nir_op_f2u16
>>>  * nir_op_f2f16
>> Subject says "...to/from 16-bit types", it should only say "to", right?
>>
>> I thought conversion from 16-bits to 32-bits was also needed but apparently
>> not (all the promotions seem to happen case by case in the backend. For 
>> example,
>> the move from 16-bits to 32-bits when 16-bit RT isn't supported). A few
>> words here in the commit would be nice explaining why only one direction is
>> needed.
> Right, I forgot, conversions in the other direction use simply f2* with source
> bit-size set accordingly.
>
> So, just drop "from" in the subject.
Just fixed the subject locally. At the early stages of the
implementation we started dealing with the 16->32 bit conversions but as
there were not alignment restrictions in this direction they are managed
just having the suitable bit-size in the source of the MOV. In this v3
series we removed some unnecessary alignments.

Thanks for devoting time to this review.

>
>>> Reviewed-by: Jason Ekstrand 
>>> ---
>>>  src/compiler/nir/nir_opcodes_c.py | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/src/compiler/nir/nir_opcodes_c.py 
>>> b/src/compiler/nir/nir_opcodes_c.py
>>> index a1db54f05a..02bb4738ed 100644
>>> --- a/src/compiler/nir/nir_opcodes_c.py
>>> +++ b/src/compiler/nir/nir_opcodes_c.py
>>> @@ -62,7 +62,7 @@ nir_type_conversion_op(nir_alu_type src, nir_alu_type dst)
>>>  % endif
>>>  %  endif
>>> switch (dst_bit_size) {
>>> -% for dst_bits in [32, 64]:
>>> +% for dst_bits in [16, 32, 64]:
>>>case ${dst_bits}:
>>>   return ${'nir_op_{0}2{1}{2}'.format(src_t[0], 
>>> dst_t[0], dst_bits)};
>>>  % endfor
>>> -- 
>>> 2.13.6
>>>
>>> ___
>>> 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

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

Re: [Mesa-dev] [PATCH v3 05/43] nir: Populate conversion opcodes to/from 16-bit types

[Pohjolainen, Topi]

On Sat, Oct 21, 2017 at 11:22:45AM +0300, Pohjolainen, Topi wrote:
> On Thu, Oct 12, 2017 at 08:37:54PM +0200, Jose Maria Casanova Crespo wrote:
> > From: Eduardo Lima Mitev 
> > 
> > This will include the following NIR ALU opcodes:
> >  * nir_op_i2i16
> >  * nir_op_i2f16
> >  * nir_op_u2u16
> >  * nir_op_u2f16
> >  * nir_op_f2i16
> >  * nir_op_f2u16
> >  * nir_op_f2f16
> 
> Subject says "...to/from 16-bit types", it should only say "to", right?
> 
> I thought conversion from 16-bits to 32-bits was also needed but apparently
> not (all the promotions seem to happen case by case in the backend. For 
> example,
> the move from 16-bits to 32-bits when 16-bit RT isn't supported). A few
> words here in the commit would be nice explaining why only one direction is
> needed.

Right, I forgot, conversions in the other direction use simply f2* with source
bit-size set accordingly.

So, just drop "from" in the subject.

> 
> > 
> > Reviewed-by: Jason Ekstrand 
> > ---
> >  src/compiler/nir/nir_opcodes_c.py | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/compiler/nir/nir_opcodes_c.py 
> > b/src/compiler/nir/nir_opcodes_c.py
> > index a1db54f05a..02bb4738ed 100644
> > --- a/src/compiler/nir/nir_opcodes_c.py
> > +++ b/src/compiler/nir/nir_opcodes_c.py
> > @@ -62,7 +62,7 @@ nir_type_conversion_op(nir_alu_type src, nir_alu_type dst)
> >  % endif
> >  %  endif
> > switch (dst_bit_size) {
> > -% for dst_bits in [32, 64]:
> > +% for dst_bits in [16, 32, 64]:
> >case ${dst_bits}:
> >   return ${'nir_op_{0}2{1}{2}'.format(src_t[0], 
> > dst_t[0], dst_bits)};
> >  % endfor
> > -- 
> > 2.13.6
> > 
> > ___
> > 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 v3 05/43] nir: Populate conversion opcodes to/from 16-bit types

[Pohjolainen, Topi]

On Thu, Oct 12, 2017 at 08:37:54PM +0200, Jose Maria Casanova Crespo wrote:
> From: Eduardo Lima Mitev 
> 
> This will include the following NIR ALU opcodes:
>  * nir_op_i2i16
>  * nir_op_i2f16
>  * nir_op_u2u16
>  * nir_op_u2f16
>  * nir_op_f2i16
>  * nir_op_f2u16
>  * nir_op_f2f16

Subject says "...to/from 16-bit types", it should only say "to", right?

I thought conversion from 16-bits to 32-bits was also needed but apparently
not (all the promotions seem to happen case by case in the backend. For example,
the move from 16-bits to 32-bits when 16-bit RT isn't supported). A few
words here in the commit would be nice explaining why only one direction is
needed.

> 
> Reviewed-by: Jason Ekstrand 
> ---
>  src/compiler/nir/nir_opcodes_c.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/compiler/nir/nir_opcodes_c.py 
> b/src/compiler/nir/nir_opcodes_c.py
> index a1db54f05a..02bb4738ed 100644
> --- a/src/compiler/nir/nir_opcodes_c.py
> +++ b/src/compiler/nir/nir_opcodes_c.py
> @@ -62,7 +62,7 @@ nir_type_conversion_op(nir_alu_type src, nir_alu_type dst)
>  % endif
>  %  endif
> switch (dst_bit_size) {
> -% for dst_bits in [32, 64]:
> +% for dst_bits in [16, 32, 64]:
>case ${dst_bits}:
>   return ${'nir_op_{0}2{1}{2}'.format(src_t[0], dst_t[0], 
> dst_bits)};
>  % endfor
> -- 
> 2.13.6
> 
> ___
> 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

[Mesa-dev] [PATCH v3 05/43] nir: Populate conversion opcodes to/from 16-bit types

[Jose Maria Casanova Crespo]

From: Eduardo Lima Mitev 

This will include the following NIR ALU opcodes:
 * nir_op_i2i16
 * nir_op_i2f16
 * nir_op_u2u16
 * nir_op_u2f16
 * nir_op_f2i16
 * nir_op_f2u16
 * nir_op_f2f16

Reviewed-by: Jason Ekstrand 
---
 src/compiler/nir/nir_opcodes_c.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/compiler/nir/nir_opcodes_c.py 
b/src/compiler/nir/nir_opcodes_c.py
index a1db54f05a..02bb4738ed 100644
--- a/src/compiler/nir/nir_opcodes_c.py
+++ b/src/compiler/nir/nir_opcodes_c.py
@@ -62,7 +62,7 @@ nir_type_conversion_op(nir_alu_type src, nir_alu_type dst)
 % endif
 %  endif
switch (dst_bit_size) {
-% for dst_bits in [32, 64]:
+% for dst_bits in [16, 32, 64]:
   case ${dst_bits}:
  return ${'nir_op_{0}2{1}{2}'.format(src_t[0], dst_t[0], 
dst_bits)};
 % endfor
-- 
2.13.6

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