[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 <elima at igalia.com>
>>>
>>> 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 <jason at jlekstrand.net>
>>> ---
>>>  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 at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev