[Mesa-dev] [PATCH v2 39/53] intel/compiler: add a helper to do conversions between integer and half-float
Francisco Jerez
currojerez at riseup.net
Fri Jan 4 10:02:49 UTC 2019
Iago Toral <itoral at igalia.com> writes:
> On Wed, 2019-01-02 at 15:00 -0800, Francisco Jerez wrote:
>> Iago Toral Quiroga <itoral at 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 at lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 227 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20190104/697ff77c/attachment.sig>
More information about the mesa-dev
mailing list