Iago Toral <ito...@igalia.com> writes: > 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. >
It's not really a bug, you just need to add a case to has_dst_aligned_region_restriction() for it to return true for FP16 instructions with this restriction, sorry I didn't point that out before. > 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
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
