[Mesa-dev] [PATCH] i965: Make exec_size 16 word/byte registers use exec_size halving again.

[Connor Abbott]

My understanding is that compression isn't necessary here, at least on
newer gens (I don't know much about gen4/5). Could you explain why a
<16,16,1>:w region is illegal? It would be nice to get a PRM citation
in the comment below.

On Fri, May 13, 2016 at 3:02 AM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> In a5d7e144eaf43fee37e6ff9e2de194407087632b, Connor generalized the
> exec_size halving code to handle more cases.  However, he accidentally
> made exec_size 16 instructions with word/byte types skip the halving
> code, producing invalid regions, and regressing a lot of Piglit tests
> on some 965GM systems.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95370
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> Cc: Connor Abbott <cwabbott0 at gmail.com>
> Cc: Francisco Jerez <currojerez at riseup.net>
> Cc: Matt Turner <mattst88 at gmail.com>
> ---
>  src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> index 4f6f3a3..383450b 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> @@ -65,7 +65,8 @@ brw_reg_from_fs_reg(fs_inst *inst, fs_reg *reg, unsigned gen)
>     case VGRF:
>        if (reg->stride == 0) {
>           brw_reg = brw_vec1_reg(brw_file_from_reg(reg), reg->nr, 0);
> -      } else if (inst->exec_size * reg->stride * type_sz(reg->type) <= 32) {
> +      } else if (inst->exec_size <= 8 &&
> +                 inst->exec_size * reg->stride * type_sz(reg->type) <= 32) {
>           brw_reg = brw_vecn_reg(inst->exec_size, brw_file_from_reg(reg),
>                                  reg->nr, 0);
>           brw_reg = stride(brw_reg, inst->exec_size * reg->stride,
> --
> 2.8.2
>