[Mesa-dev] [PATCH 2/5] i965/fs: fix stride and type for hw_reg's in regs_read()

[Francisco Jerez]

Connor Abbott <cwabbott0 at gmail.com> writes:

> sources with file == HW_REG get all their information from the
> fixed_hw_reg field, so we need to get the stride and type from there
> when computing the size.
>
> Signed-off-by: Connor Abbott <connor.w.abbott at intel.com>
> ---
>  src/mesa/drivers/dri/i965/brw_fs.cpp | 24 ++++++++++++++++++------
>  1 file changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 38b9095..64f093b 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -696,24 +696,36 @@ fs_inst::regs_read(int arg) const
>        break;
>     }
>  
> +   unsigned stride;
> +   enum brw_reg_type type;
> +
>     switch (src[arg].file) {
>     case BAD_FILE:
>     case UNIFORM:
>     case IMM:
>        return 1;
> +
>     case GRF:
> +      stride = src[arg].stride;
> +      type = src[arg].type;
> +      break;
> +
>     case HW_REG:
> -      if (src[arg].stride == 0) {
> -         return 1;
> -      } else {
> -         int size = components * this->exec_size * type_sz(src[arg].type);
> -         return DIV_ROUND_UP(size * src[arg].stride, 32);
> -      }
> +      stride = src[arg].fixed_hw_reg.hstride;
> +      type = src[arg].fixed_hw_reg.type;
> +      break;
> +
>     case MRF:
>        unreachable("MRF registers are not allowed as sources");
>     default:
>        unreachable("Invalid register file");
>     }
> +
> +   if (stride == 0)
> +      return 1;
> +
> +   int size = components * this->exec_size * type_sz(type);
> +   return DIV_ROUND_UP(size * stride, 32);

I don't think this will work unfortunately, brw_reg::hstride is the log2
of the actual stride, unlike fs_reg::stride.  Did I already mention I'm
appalled by the fact that fs_reg has a number of fields with overlapping
semantics but different representation, one or the other being
applicable depending on the occasion.  I guess it would be more or less
bearable if these data members were declared private and some reasonable
abstraction was provided to access them.

How do you like the attached patch?  It doesn't solve the fundamental
problem but it seems to improve the situation slightly.

>  }
>  
>  bool
> -- 
> 2.4.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-i965-fs-Factor-out-universally-broken-calculation-of.patch
Type: text/x-diff
Size: 4751 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150714/4e0bd2d2/attachment-0001.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150714/4e0bd2d2/attachment-0001.sig>