On Wed, 2019-01-02 at 15:00 -0800, Francisco Jerez wrote: > Iago Toral Quiroga <ito...@igalia.com> writes: > > > There are hardware restrictions to consider that seem to affect > > atom platforms > > only. > > Same comment here as for PATCH 13 of this series. This and PATCH 40 > shouldn't be necessary anymore with [1] in place. Please drop them.
Actually, I think your pass doesn't handle this case. I have just tested this and I get various regressions for conversions between integers (specifically integers lower than 32-bit, so I wonder if this restriction only affects these cases) and half-float. Here is an example of final IR generated with your pass and without the call to fixup_int_half_float_conversion from my series: mov(8) vgrf1+0.0:HF, vgrf0<2>:W Which should be: mov(8) vgrf1<2>:HF, vgrf0<2>:W It seems your pass doesn't act on this code since INTEL_DEBUG=optimizer doesn't show any trace of it. If this is not a bug in your pass and just that it doesn't handle this case, I am happy to add the support for it as part of my series if that makes sense to you, just let me know if that is the case. Iago > [1] > https://lists.freedesktop.org/archives/mesa-dev/2018-December/212775.html > > > --- > > src/intel/compiler/brw_fs_nir.cpp | 32 > > +++++++++++++++++++++++++++++++ > > 1 file changed, 32 insertions(+) > > > > diff --git a/src/intel/compiler/brw_fs_nir.cpp > > b/src/intel/compiler/brw_fs_nir.cpp > > index 802f5cb0944..a9fd98bab68 100644 > > --- a/src/intel/compiler/brw_fs_nir.cpp > > +++ b/src/intel/compiler/brw_fs_nir.cpp > > @@ -696,6 +696,38 @@ fixup_64bit_conversion(const fs_builder &bld, > > return false; > > } > > > > +static bool > > +fixup_int_half_float_conversion(const fs_builder &bld, > > + fs_reg dst, fs_reg src, > > + bool saturate, > > + const struct gen_device_info > > *devinfo) > > +{ > > + /* 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)) > > + return false; > > + > > + if (!((dst.type == BRW_REGISTER_TYPE_HF && > > !brw_reg_type_is_floating_point(src.type)) || > > + (src.type == BRW_REGISTER_TYPE_HF && > > !brw_reg_type_is_floating_point(dst.type)))) > > + return false; > > + > > + fs_reg tmp = horiz_stride(retype(bld.vgrf(BRW_REGISTER_TYPE_F, > > 1), > > + dst.type), > > + 2); > > + bld.MOV(tmp, src); > > + fs_inst *inst = bld.MOV(dst, tmp); > > + inst->saturate = saturate; > > + > > + return true; > > +} > > + > > void > > fs_visitor::nir_emit_alu(const fs_builder &bld, nir_alu_instr > > *instr) > > { > > -- > > 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
