[Mesa-dev] [PATCH 15/24] i965/fs: Set stride = 1 for vector immediate types.

[Kenneth Graunke]

On Monday, November 02, 2015 04:29:25 PM Matt Turner wrote:
> The generator asserts that this is true (and presumably it's useful in
> some optimization passes?) and the VF fs_reg constructors did this (by
> virtue of the fact that it doesn't override what init() does).
> 
> In the next commit, calling this constructor with brw_imm_* will
> generate an IMM file register rather than a HW_REG, making this change
> necessary to avoid breakage with existing uses of brw_imm_v().
> ---
>  src/mesa/drivers/dri/i965/brw_fs.cpp | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 91eaf61..92a9437 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -427,6 +427,12 @@ fs_reg::fs_reg(struct brw_reg reg) :
>     this->subreg_offset = 0;
>     this->reladdr = NULL;
>     this->stride = 1;
> +   if (this->file == IMM &&
> +       (this->type != BRW_REGISTER_TYPE_V &&
> +        this->type != BRW_REGISTER_TYPE_UV &&
> +        this->type != BRW_REGISTER_TYPE_VF)) {
> +      this->stride = 0;
> +   }
>  }
>  
>  bool
> 

It's confusing that your subject says "Set stride to 1 for vector
immediate types" yet your code sets stride to 0.

I would suggest renaming the patch to

"i965/fs: Set stride correctly for immediates in fs_reg(brw_reg)"

and adding some text to the commit message like:

"The fs_reg() constructors for immediates set stride to 0, except for
 vector-immediates, which set stride to 1.  This patch makes the fs_reg
 constructor that takes a brw_reg do likewise, so that stride is set
 correctly for cases such as fs_reg(brw_imm_v(...))."

Regardless,
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20151111/70c32111/attachment.sig>