Re: [Mesa-dev] [PATCH v4 23/40] intel/compiler: rework conversion opcodes

2019-02-16 Thread Jason Ekstrand
Reviewed-by: Jason Ekstrand 

On Tue, Feb 12, 2019 at 5:57 AM Iago Toral Quiroga 
wrote:

> Now that we have the regioning lowering pass we can just put all of these
> opcodes together in a single block and we can just assert on the few cases
> of conversion instructions that are not supported in hardware and that
> should
> be lowered in brw_nir_lower_conversions.
>
> The only cases what we still handle separately are the conversions from
> float
> to half-float since the rounding variants would need to fallthrough and we
> are already doing this for boolean opcodes (since they need to negate),
> plus
> there is also a large comment about these opcodes that we probably want to
> keep so it is just easier to keep these separate.
>
> Suggested-by: Jason Ekstrand 
> ---
>  src/intel/compiler/brw_fs_nir.cpp | 41 +--
>  1 file changed, 22 insertions(+), 19 deletions(-)
>
> diff --git a/src/intel/compiler/brw_fs_nir.cpp
> b/src/intel/compiler/brw_fs_nir.cpp
> index f59e9ad4e2b..3a6e4a2eb60 100644
> --- a/src/intel/compiler/brw_fs_nir.cpp
> +++ b/src/intel/compiler/brw_fs_nir.cpp
> @@ -772,7 +772,7 @@ fs_visitor::nir_emit_alu(const fs_builder &bld,
> nir_alu_instr *instr)
>bld.emit(SHADER_OPCODE_RND_MODE, bld.null_reg_ud(),
> brw_imm_d(brw_rnd_mode_from_nir_op(instr->op)));
>/* fallthrough */
> -
> +   case nir_op_f2f16:
>/* 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.
> @@ -782,23 +782,11 @@ fs_visitor::nir_emit_alu(const fs_builder &bld,
> nir_alu_instr *instr)
> * 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_i2f16:
> -   case nir_op_u2f16:
>assert(type_sz(op[0].type) < 8); /* brw_nir_lower_conversions */
>inst = bld.MOV(result, op[0]);
>inst->saturate = instr->dest.saturate;
>break;
>
> -   case nir_op_f2f64:
> -   case nir_op_f2i64:
> -   case nir_op_f2u64:
> -  assert(type_sz(op[0].type) > 2); /* brw_nir_lower_conversions */
> -  inst = bld.MOV(result, op[0]);
> -  inst->saturate = instr->dest.saturate;
> -  break;
> -
> case nir_op_b2i8:
> case nir_op_b2i16:
> case nir_op_b2i32:
> @@ -813,19 +801,34 @@ fs_visitor::nir_emit_alu(const fs_builder &bld,
> nir_alu_instr *instr)
> case nir_op_i2i64:
> case nir_op_u2f64:
> case nir_op_u2u64:
> -  assert(type_sz(op[0].type) > 1); /* brw_nir_lower_conversions */
> -  /* fallthrough */
> +   case nir_op_f2f64:
> +   case nir_op_f2i64:
> +   case nir_op_f2u64:
> +   case nir_op_i2i32:
> +   case nir_op_u2u32:
> case nir_op_f2f32:
> case nir_op_f2i32:
> case nir_op_f2u32:
> -   case nir_op_f2i16:
> -   case nir_op_f2u16:
> -   case nir_op_i2i32:
> -   case nir_op_u2u32:
> +   case nir_op_i2f16:
> case nir_op_i2i16:
> +   case nir_op_u2f16:
> case nir_op_u2u16:
> +   case nir_op_f2i16:
> +   case nir_op_f2u16:
> case nir_op_i2i8:
> case nir_op_u2u8:
> +   case nir_op_f2i8:
> +   case nir_op_f2u8:
> +  if (result.type == BRW_REGISTER_TYPE_B ||
> +  result.type == BRW_REGISTER_TYPE_UB ||
> +  result.type == BRW_REGISTER_TYPE_HF)
> + assert(type_sz(op[0].type) < 8); /* brw_nir_lower_conversions */
> +
> +  if (op[0].type == BRW_REGISTER_TYPE_B ||
> +  op[0].type == BRW_REGISTER_TYPE_UB ||
> +  op[0].type == BRW_REGISTER_TYPE_HF)
> + assert(type_sz(result.type) < 8); /* brw_nir_lower_conversions */
> +
>inst = bld.MOV(result, op[0]);
>inst->saturate = instr->dest.saturate;
>break;
> --
> 2.17.1
>
> ___
> 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 v4 23/40] intel/compiler: rework conversion opcodes

2019-02-12 Thread Iago Toral Quiroga
Now that we have the regioning lowering pass we can just put all of these
opcodes together in a single block and we can just assert on the few cases
of conversion instructions that are not supported in hardware and that should
be lowered in brw_nir_lower_conversions.

The only cases what we still handle separately are the conversions from float
to half-float since the rounding variants would need to fallthrough and we
are already doing this for boolean opcodes (since they need to negate), plus
there is also a large comment about these opcodes that we probably want to
keep so it is just easier to keep these separate.

Suggested-by: Jason Ekstrand 
---
 src/intel/compiler/brw_fs_nir.cpp | 41 +--
 1 file changed, 22 insertions(+), 19 deletions(-)

diff --git a/src/intel/compiler/brw_fs_nir.cpp 
b/src/intel/compiler/brw_fs_nir.cpp
index f59e9ad4e2b..3a6e4a2eb60 100644
--- a/src/intel/compiler/brw_fs_nir.cpp
+++ b/src/intel/compiler/brw_fs_nir.cpp
@@ -772,7 +772,7 @@ fs_visitor::nir_emit_alu(const fs_builder &bld, 
nir_alu_instr *instr)
   bld.emit(SHADER_OPCODE_RND_MODE, bld.null_reg_ud(),
brw_imm_d(brw_rnd_mode_from_nir_op(instr->op)));
   /* fallthrough */
-
+   case nir_op_f2f16:
   /* 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.
@@ -782,23 +782,11 @@ fs_visitor::nir_emit_alu(const fs_builder &bld, 
nir_alu_instr *instr)
* 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_i2f16:
-   case nir_op_u2f16:
   assert(type_sz(op[0].type) < 8); /* brw_nir_lower_conversions */
   inst = bld.MOV(result, op[0]);
   inst->saturate = instr->dest.saturate;
   break;
 
-   case nir_op_f2f64:
-   case nir_op_f2i64:
-   case nir_op_f2u64:
-  assert(type_sz(op[0].type) > 2); /* brw_nir_lower_conversions */
-  inst = bld.MOV(result, op[0]);
-  inst->saturate = instr->dest.saturate;
-  break;
-
case nir_op_b2i8:
case nir_op_b2i16:
case nir_op_b2i32:
@@ -813,19 +801,34 @@ fs_visitor::nir_emit_alu(const fs_builder &bld, 
nir_alu_instr *instr)
case nir_op_i2i64:
case nir_op_u2f64:
case nir_op_u2u64:
-  assert(type_sz(op[0].type) > 1); /* brw_nir_lower_conversions */
-  /* fallthrough */
+   case nir_op_f2f64:
+   case nir_op_f2i64:
+   case nir_op_f2u64:
+   case nir_op_i2i32:
+   case nir_op_u2u32:
case nir_op_f2f32:
case nir_op_f2i32:
case nir_op_f2u32:
-   case nir_op_f2i16:
-   case nir_op_f2u16:
-   case nir_op_i2i32:
-   case nir_op_u2u32:
+   case nir_op_i2f16:
case nir_op_i2i16:
+   case nir_op_u2f16:
case nir_op_u2u16:
+   case nir_op_f2i16:
+   case nir_op_f2u16:
case nir_op_i2i8:
case nir_op_u2u8:
+   case nir_op_f2i8:
+   case nir_op_f2u8:
+  if (result.type == BRW_REGISTER_TYPE_B ||
+  result.type == BRW_REGISTER_TYPE_UB ||
+  result.type == BRW_REGISTER_TYPE_HF)
+ assert(type_sz(op[0].type) < 8); /* brw_nir_lower_conversions */
+
+  if (op[0].type == BRW_REGISTER_TYPE_B ||
+  op[0].type == BRW_REGISTER_TYPE_UB ||
+  op[0].type == BRW_REGISTER_TYPE_HF)
+ assert(type_sz(result.type) < 8); /* brw_nir_lower_conversions */
+
   inst = bld.MOV(result, op[0]);
   inst->saturate = instr->dest.saturate;
   break;
-- 
2.17.1

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