Yes, I mentioned this in another of my replies. I think we probably
want a combination of these things. For some restrictions like the
cases where we need to split a conversion in two instructions going
through an intermediate type I think a NIR lowering would be good, but
then I think we might also want to create one or two specialized
conversion helpers in the backend that deal with specific groups of
situations such as maybe conversions to/from 64-bit or to/from lower
bit-sizes, or maybe to/from integer, etc I need to think a bit more
about what would be a good strategy, but in any case, I share your view
of the situation and I agree we should do something about it.
Iago
On Fri, 2018-12-07 at 13:53 -0600, Jason Ekstrand wrote:
> We are starting to get a *lot* of special cases in the conversion
> code. I'm not sure what the best thing to do is. Maybe some master
> conversion function that just does it all? Maybe some NIR lowering?
> In any case, I think we can do better than the pile of special cases
> we are starting to accumulate.
>
> On Tue, Dec 4, 2018 at 1:18 AM Iago Toral Quiroga <ito...@igalia.com>
> wrote:
> > Section Register Region Restriction of the 3D Media GPGPU chapter
> > states:
> >
> >
> >
> > "Conversion between Integer and HF (Half Float) must be DWord
> >
> > aligned and strided by a DWord on the destination."
> >
> >
> >
> > The same restriction shows up in all hardware platforms that
> > support
> >
> > half-float, however, empirical testing suggests that only atom
> >
> > platforms are affected.
> >
> > ---
> >
> > src/intel/compiler/brw_fs_nir.cpp | 41
> > +++++++++++++++++++++++++++++--
> >
> > 1 file changed, 39 insertions(+), 2 deletions(-)
> >
> >
> >
> > diff --git a/src/intel/compiler/brw_fs_nir.cpp
> > b/src/intel/compiler/brw_fs_nir.cpp
> >
> > index 3f98c6a4474..db3a8812ae3 100644
> >
> > --- a/src/intel/compiler/brw_fs_nir.cpp
> >
> > +++ b/src/intel/compiler/brw_fs_nir.cpp
> >
> > @@ -917,6 +917,25 @@ fs_visitor::nir_emit_alu(const fs_builder
> > &bld, nir_alu_instr *instr)
> >
> > inst->saturate = instr->dest.saturate;
> >
> > break;
> >
> > }
> >
> > +
> >
> > + /* CHV PRM, 3D Media GPGPU Engine, Register Region
> > Restrictions,
> >
> > + * Special Restrictions:
> >
> > + *
> >
> > + * "Conversion between Integer and HF (Half Float) must
> > be DWord
> >
> > + * aligned and strided by a DWord on the destination."
> >
> > + *
> >
> > + * The same restriction is listed for other hardware
> > platforms, however,
> >
> > + * empirical testing suggests that only atom platforms are
> > affected.
> >
> > + */
> >
> > + if ((devinfo->is_cherryview ||
> > gen_device_info_is_9lp(devinfo)) &&
> >
> > + nir_dest_bit_size(instr->dest.dest) == 16) {
> >
> > + assert(result.type == BRW_REGISTER_TYPE_HF);
> >
> > + fs_reg tmp =
> >
> > + horiz_stride(retype(bld.vgrf(BRW_REGISTER_TYPE_F, 1),
> > result.type), 2);
> >
> > + bld.MOV(tmp, op[0]);
> >
> > + op[0] = tmp;
> >
> > + }
> >
> > +
> >
> > inst = bld.MOV(result, op[0]);
> >
> > inst->saturate = instr->dest.saturate;
> >
> > break;
> >
> > @@ -939,11 +958,29 @@ fs_visitor::nir_emit_alu(const fs_builder
> > &bld, nir_alu_instr *instr)
> >
> > }
> >
> > /* Fallthrough */
> >
> >
> >
> > + case nir_op_f2i16:
> >
> > + case nir_op_f2u16:
> >
> > + /* CHV PRM, 3D Media GPGPU Engine, Register Region
> > Restrictions,
> >
> > + * Special Restrictions:
> >
> > + *
> >
> > + * "Conversion between Integer and HF (Half Float) must
> > be DWord
> >
> > + * aligned and strided by a DWord on the destination."
> >
> > + *
> >
> > + * The same restriction is listed for other hardware
> > platforms, however,
> >
> > + * empirical testing suggests that only atom platforms are
> > affected.
> >
> > + */
> >
> > + if ((devinfo->is_cherryview ||
> > gen_device_info_is_9lp(devinfo)) &&
> >
> > + nir_src_bit_size(instr->src[0].src) == 16) {
> >
> > + fs_reg tmp =
> >
> > + horiz_stride(retype(bld.vgrf(BRW_REGISTER_TYPE_D, 1),
> > result.type), 2);
> >
> > + bld.MOV(tmp, op[0]);
> >
> > + op[0] = tmp;
> >
> > + }
> >
> > + /* Fallthrough */
> >
> > +
> >
> > 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_i2i16:
> >
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
