On Wed, Jan 23, 2019 at 4:18 AM Iago Toral Quiroga <ito...@igalia.com> wrote:
>
> Commit c84ec70b3a72 implemented execution type promotion to 32-bit for
> conversions involving half-float registers, which empirical testing suggested
> was required, but it did not incorporate this change into the assembly 
> validator
> logic. This commits adds that, preventing validation errors like this:
>
> mov(16)  g9<4>B   g3<16,8,2>HF     { align1 1H };
> ERROR: Destination stride must be equal to the ratio of the sizes of the
>        execution data type to the destination type
>
> Fixes: c84ec70b3a72 "intel/fs: Promote execution type to 32-bit when any 
> half-float conversion is needed."
> ---
>  src/intel/compiler/brw_eu_validate.c | 27 ++++++++++++++-------------

New rule: New restrictions (or relaxations) may not be added to
brw_eu_validate.c without accompanying unit tests. I'll send a patch
to add a comment to brw_eu_validate.c saying as much.

Rationale: the reason I wrote brw_eu_validate.c was because I wasted a
week debugging an issue where fulsim not only failed to inform me that
one instruction was invalid but also incorrectly told me that one
correct instruction *was* invalid. I would have been better off
without such a tool.

If the EU validator loses people's trust, then it's useless, but if it
is incorrect it's worse than useless.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to