On January 18, 2019 06:23:35 Iago Toral <ito...@igalia.com> wrote:
On Fri, 2019-01-18 at 12:13 +0100, Iago Toral wrote:
On Thu, 2019-01-17 at 17:12 -0600, Jason Ekstrand wrote:
This patch doesn't really do what the commit message says. What it really does is implement float -> 8-bit converions for *any* size float.

On Tue, Jan 15, 2019 at 7:55 AM Iago Toral Quiroga <ito...@igalia.com> wrote:
These are not directly supported in hardware and brw_nir_lower_conversions
should have taken care of that before we get here. Also, while we are
at it, make sure 64-bit integer to 8-bit are also properly split by
the same lowering pass, since they have the same hardware restrictions.

Now that we have a lowering pass, having separate cases just so one of them can assert seems silly. If anything, we should just do

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 */

and have it all in one big case. The only special case we need is for booleans where we need to negate them and fall through.

There are more cases, since the inverse conversions of these are not supported either. I guess I'll just add this as well:

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 */

Oh, and there is also the rounding opcodes for f16 destinations (plus the big comment about brw_F32TO16 that comes with their fallthrough)... I think we might want to keep the three f2f16* opcodes as a separate block and do as you suggest for the remaining ones if that is okay.

That's fine as they actually do do something different.

-Jason


--Jason

---
src/intel/compiler/brw_fs_nir.cpp | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/intel/compiler/brw_fs_nir.cpp b/src/intel/compiler/brw_fs_nir.cpp
index cf546b8ff09..e454578d99b 100644
--- a/src/intel/compiler/brw_fs_nir.cpp
+++ b/src/intel/compiler/brw_fs_nir.cpp
@@ -786,6 +786,10 @@ fs_visitor::nir_emit_alu(const fs_builder &bld, nir_alu_instr *instr)
   case nir_op_f2f16:
   case nir_op_i2f16:
   case nir_op_u2f16:
+   case nir_op_i2i8:
+   case nir_op_u2u8:
+   case nir_op_f2i8:
+   case nir_op_f2u8:
      assert(type_sz(op[0].type) < 8); /* brw_nir_lower_conversions */
      inst = bld.MOV(result, op[0]);
      inst->saturate = instr->dest.saturate;
@@ -824,8 +828,6 @@ fs_visitor::nir_emit_alu(const fs_builder &bld, nir_alu_instr *instr)
   case nir_op_u2u32:
   case nir_op_i2i16:
   case nir_op_u2u16:
-   case nir_op_i2i8:
-   case nir_op_u2u8:
      inst = bld.MOV(result, op[0]);
      inst->saturate = instr->dest.saturate;
      break;

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

Reply via email to