Re: [Mesa-dev] [PATCH v3 14/43] i965/fs: Handle 32-bit to 16-bit conversions

[Chema Casanova]

On 14/10/17 09:55, Pohjolainen, Topi wrote:
> On Thu, Oct 12, 2017 at 08:38:03PM +0200, Jose Maria Casanova Crespo wrote:
>> From: Alejandro Piñeiro 
>>
>> Conversions to 16-bit need having aligment between the 16-bit
>> and 32-bit types. So the conversion operations unpack 16-bit types
>> to with an stride=2 and then applies a MOV with the conversion.
>>
>> v2 (Jason Ekstrand):
>>   - Avoid the general use of stride=2 for 16-bit register types.
>>
>> Signed-off-by: Eduardo Lima 
>> Signed-off-by: Alejandro Piñeiro 
>> Signed-off-by: Jose Maria Casanova Crespo 
>> ---
>>  src/intel/compiler/brw_fs_nir.cpp | 25 +
>>  1 file changed, 25 insertions(+)
>>
>> diff --git a/src/intel/compiler/brw_fs_nir.cpp 
>> b/src/intel/compiler/brw_fs_nir.cpp
>> index affe65d5e9..6908c7ea02 100644
>> --- a/src/intel/compiler/brw_fs_nir.cpp
>> +++ b/src/intel/compiler/brw_fs_nir.cpp
>> @@ -693,6 +693,31 @@ fs_visitor::nir_emit_alu(const fs_builder , 
>> nir_alu_instr *instr)
>>inst->saturate = instr->dest.saturate;
>>break;
>>  
>> +  /* In theory, it would be better to use BRW_OPCODE_F32TO16. Depending
>> +   * on the HW gen, it is a special hw opcode or just a MOV, and
>> +   * brw_F32TO16 (at brw_eu_emit) would do the work to chose.
>> +   *
>> +   * But if we want to use that opcode, we need to provide support on
>> +   * different optimizations and lowerings. As right now HF support is
>> +   * only for gen8+, it will be better to use directly the MOV, and use
>> +   * BRW_OPCODE_F32TO16 when/if we work for HF support on gen7.
>> +   */
>> +
>> +   case nir_op_f2f16:
>> +   case nir_op_i2i16:
>> +   case nir_op_u2u16: {
>> +  /* TODO: Fixing aligment rules for conversions from 32-bits to
>> +   * 16-bit types should be moved to lower_conversions
>> +   */
>> +  fs_reg tmp = bld.vgrf(op[0].type, 1);
>> +  tmp = subscript(tmp, result.type, 0);
>> +  inst = bld.MOV(tmp, op[0]);
>> +  inst->saturate = instr->dest.saturate;
>> +  inst = bld.MOV(result ,tmp);
> 
> Move space after ','
> 

Fixed locally.

Thanks.

>> +  inst->saturate = instr->dest.saturate;
>> +  break;
>> +   }
>> +
>> case nir_op_f2f64:
>> case nir_op_i2f64:
>> case nir_op_u2f64:
>> -- 
>> 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 14/43] i965/fs: Handle 32-bit to 16-bit conversions

[Pohjolainen, Topi]

On Thu, Oct 12, 2017 at 08:38:03PM +0200, Jose Maria Casanova Crespo wrote:
> From: Alejandro Piñeiro 
> 
> Conversions to 16-bit need having aligment between the 16-bit
> and 32-bit types. So the conversion operations unpack 16-bit types
> to with an stride=2 and then applies a MOV with the conversion.
> 
> v2 (Jason Ekstrand):
>   - Avoid the general use of stride=2 for 16-bit register types.
> 
> Signed-off-by: Eduardo Lima 
> Signed-off-by: Alejandro Piñeiro 
> Signed-off-by: Jose Maria Casanova Crespo 
> ---
>  src/intel/compiler/brw_fs_nir.cpp | 25 +
>  1 file changed, 25 insertions(+)
> 
> diff --git a/src/intel/compiler/brw_fs_nir.cpp 
> b/src/intel/compiler/brw_fs_nir.cpp
> index affe65d5e9..6908c7ea02 100644
> --- a/src/intel/compiler/brw_fs_nir.cpp
> +++ b/src/intel/compiler/brw_fs_nir.cpp
> @@ -693,6 +693,31 @@ fs_visitor::nir_emit_alu(const fs_builder , 
> nir_alu_instr *instr)
>inst->saturate = instr->dest.saturate;
>break;
>  
> +  /* In theory, it would be better to use BRW_OPCODE_F32TO16. Depending
> +   * on the HW gen, it is a special hw opcode or just a MOV, and
> +   * brw_F32TO16 (at brw_eu_emit) would do the work to chose.
> +   *
> +   * But if we want to use that opcode, we need to provide support on
> +   * different optimizations and lowerings. As right now HF support is
> +   * only for gen8+, it will be better to use directly the MOV, and use
> +   * BRW_OPCODE_F32TO16 when/if we work for HF support on gen7.
> +   */
> +
> +   case nir_op_f2f16:
> +   case nir_op_i2i16:
> +   case nir_op_u2u16: {
> +  /* TODO: Fixing aligment rules for conversions from 32-bits to
> +   * 16-bit types should be moved to lower_conversions
> +   */
> +  fs_reg tmp = bld.vgrf(op[0].type, 1);
> +  tmp = subscript(tmp, result.type, 0);
> +  inst = bld.MOV(tmp, op[0]);
> +  inst->saturate = instr->dest.saturate;
> +  inst = bld.MOV(result ,tmp);

Move space after ','

> +  inst->saturate = instr->dest.saturate;
> +  break;
> +   }
> +
> case nir_op_f2f64:
> case nir_op_i2f64:
> case nir_op_u2f64:
> -- 
> 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 14/43] i965/fs: Handle 32-bit to 16-bit conversions

[Jose Maria Casanova Crespo]

From: Alejandro Piñeiro 

Conversions to 16-bit need having aligment between the 16-bit
and 32-bit types. So the conversion operations unpack 16-bit types
to with an stride=2 and then applies a MOV with the conversion.

v2 (Jason Ekstrand):
  - Avoid the general use of stride=2 for 16-bit register types.

Signed-off-by: Eduardo Lima 
Signed-off-by: Alejandro Piñeiro 
Signed-off-by: Jose Maria Casanova Crespo 
---
 src/intel/compiler/brw_fs_nir.cpp | 25 +
 1 file changed, 25 insertions(+)

diff --git a/src/intel/compiler/brw_fs_nir.cpp 
b/src/intel/compiler/brw_fs_nir.cpp
index affe65d5e9..6908c7ea02 100644
--- a/src/intel/compiler/brw_fs_nir.cpp
+++ b/src/intel/compiler/brw_fs_nir.cpp
@@ -693,6 +693,31 @@ fs_visitor::nir_emit_alu(const fs_builder , 
nir_alu_instr *instr)
   inst->saturate = instr->dest.saturate;
   break;
 
+  /* In theory, it would be better to use BRW_OPCODE_F32TO16. Depending
+   * on the HW gen, it is a special hw opcode or just a MOV, and
+   * brw_F32TO16 (at brw_eu_emit) would do the work to chose.
+   *
+   * But if we want to use that opcode, we need to provide support on
+   * different optimizations and lowerings. As right now HF support is
+   * only for gen8+, it will be better to use directly the MOV, and use
+   * BRW_OPCODE_F32TO16 when/if we work for HF support on gen7.
+   */
+
+   case nir_op_f2f16:
+   case nir_op_i2i16:
+   case nir_op_u2u16: {
+  /* TODO: Fixing aligment rules for conversions from 32-bits to
+   * 16-bit types should be moved to lower_conversions
+   */
+  fs_reg tmp = bld.vgrf(op[0].type, 1);
+  tmp = subscript(tmp, result.type, 0);
+  inst = bld.MOV(tmp, op[0]);
+  inst->saturate = instr->dest.saturate;
+  inst = bld.MOV(result ,tmp);
+  inst->saturate = instr->dest.saturate;
+  break;
+   }
+
case nir_op_f2f64:
case nir_op_i2f64:
case nir_op_u2f64:
-- 
2.13.6

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