[Mesa-dev] [PATCH 5/9] intel/compiler: relax brw_eu_validate for byte raw movs

[Matt Turner]

On Sun, Jul 8, 2018 at 5:27 PM, Jose Maria Casanova Crespo
<jmcasanova at igalia.com> wrote:
> When the destination is a BYTE type allow raw movs
> even if the stride is not exact multiple of destination
> type and exec type, execution type is Word and its size is 2.
>
> This restriction was only allowing stride==2 destinations
> for 8-bit types.

Super late review, obviously... it's been on my todo list but fp64 was
taking all my time.

I can't figure this commit out. What I know:

 - byte destination
 - raw mov (which means destination stride == 1)
 - execution type of a byte operation is "word"

The original code

>     if (exec_type_size > dst_type_size) {
>        ERROR_IF(dst_stride * dst_type_size != exec_type_size,
>                 "Destination stride must be equal to the ratio of the sizes of "
>                 "the execution data type to the destination type");
>     }

would not have worked for an instruction like

> mov(8)          g0<1>B          g0<8,8,1>B

But, that's okay because it didn't need to since the block right above
it does this:

   if (dst_type_is_byte) {
      if (is_packed(exec_size * dst_stride, exec_size, dst_stride)) {
         if (!inst_is_raw_move(devinfo, inst)) {
            ERROR("Only raw MOV supports a packed-byte destination");
            return error_msg;
         } else {
            return (struct string){};
         }
      }
   }

That is, if it's a raw move, return no-error.

It would be easier to understand what you were fixing if you had added
a unit test to test_eu_validate.cpp, or (if my suspicions are correct)
it would have proven to you that this patch wasn't correct.

Was this just something that you noticed by inspection?

> Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>

Sigh.