[Mesa-dev] [PATCHv2] i965/fs: Factor out source components calculation to a separate method.

[Jason Ekstrand]

R-B
On Jul 27, 2015 6:12 AM, "Francisco Jerez" <currojerez at riseup.net> wrote:

> This cleans up fs_inst::regs_read() slightly by disentangling the
> calculation of "components" from the handling of message payload
> arguments.  This will also simplify the SIMD lowering and logical send
> message lowering passes, because it will avoid expressions like
> 'regs_read * REG_SIZE / component_size' which are not only ugly, they
> may be inaccurate because regs_read rounds up the result to the
> closest register multiple so they could give incorrect results when
> the component size is lower than one register (e.g. uniforms).  This
> didn't seem to be a problem right now because all such expressions
> happen to be dealing with per-channel GRFs only currently, but that's
> by no means obvious so better be safe than sorry.
>
> v2: Split PIXEL_X/Y and LINTERP into separate case blocks.
> ---
>  src/mesa/drivers/dri/i965/brw_fs.cpp  | 33
> +++++++++++++++++++++++----------
>  src/mesa/drivers/dri/i965/brw_ir_fs.h |  1 +
>  2 files changed, 24 insertions(+), 10 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 2b7ddc9..a620c79 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -663,10 +663,29 @@ fs_inst::is_partial_write() const
>             !this->dst.is_contiguous());
>  }
>
> +unsigned
> +fs_inst::components_read(unsigned i) const
> +{
> +   switch (opcode) {
> +   case FS_OPCODE_LINTERP:
> +      if (i == 0)
> +         return 2;
> +      else
> +         return 1;
> +
> +   case FS_OPCODE_PIXEL_X:
> +   case FS_OPCODE_PIXEL_Y:
> +      assert(i == 0);
> +      return 2;
> +
> +   default:
> +      return 1;
> +   }
> +}
> +
>  int
>  fs_inst::regs_read(int arg) const
>  {
> -   unsigned components = 1;
>     switch (opcode) {
>     case FS_OPCODE_FB_WRITE:
>     case SHADER_OPCODE_URB_WRITE_SIMD8:
> @@ -688,15 +707,8 @@ fs_inst::regs_read(int arg) const
>        break;
>
>     case FS_OPCODE_LINTERP:
> -      if (arg == 0)
> -         return exec_size / 4;
> -      else
> +      if (arg == 1)
>           return 1;
> -
> -   case FS_OPCODE_PIXEL_X:
> -   case FS_OPCODE_PIXEL_Y:
> -      if (arg == 0)
> -         components = 2;
>        break;
>
>     case SHADER_OPCODE_LOAD_PAYLOAD:
> @@ -720,7 +732,8 @@ fs_inst::regs_read(int arg) const
>        return 1;
>     case GRF:
>     case HW_REG:
> -      return DIV_ROUND_UP(components * src[arg].component_size(exec_size),
> +      return DIV_ROUND_UP(components_read(arg) *
> +                          src[arg].component_size(exec_size),
>                            REG_SIZE);
>     case MRF:
>        unreachable("MRF registers are not allowed as sources");
> diff --git a/src/mesa/drivers/dri/i965/brw_ir_fs.h
> b/src/mesa/drivers/dri/i965/brw_ir_fs.h
> index 693357f..97c6f8b 100644
> --- a/src/mesa/drivers/dri/i965/brw_ir_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_ir_fs.h
> @@ -201,6 +201,7 @@ public:
>     bool is_send_from_grf() const;
>     bool is_partial_write() const;
>     bool is_copy_payload(const brw::simple_allocator &grf_alloc) const;
> +   unsigned components_read(unsigned i) const;
>     int regs_read(int arg) const;
>     bool can_do_source_mods(const struct brw_device_info *devinfo);
>     bool has_side_effects() const;
> --
> 2.4.6
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150727/6da76e2b/attachment.html>