[Mesa-dev] [PATCH 4/8] i965/fs: Exit the compile if spilling would overwrite in-use MRFs.

[Paul Berry]

On 29 October 2013 13:28, Eric Anholt <eric at anholt.net> wrote:

> I believe this will never happen in SIMD8 mode, but it could for SIMD16
> when we fix it.
> ---
>  src/mesa/drivers/dri/i965/brw_fs.h                |  1 +
>  src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp | 23
> +++++++++++++++++++++++
>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp      |  2 ++
>  3 files changed, 26 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h
> b/src/mesa/drivers/dri/i965/brw_fs.h
> index 50a045e..fb1da4c 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> @@ -466,6 +466,7 @@ public:
>     fs_reg shader_start_time;
>
>     int grf_used;
> +   bool spilled_any_registers;
>
>     const unsigned dispatch_width; /**< 8 or 16 */
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp
> index d86027e..50f3414 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp
> @@ -643,6 +643,29 @@ fs_visitor::spill_reg(int spill_reg)
>     unsigned int spill_offset = c->last_scratch;
>     assert(ALIGN(spill_offset, 16) == spill_offset); /* oword read/write
> req. */
>     c->last_scratch += size * REG_SIZE;
> +   int spill_base_mrf = dispatch_width > 8 ? 13 : 14;
> +
> +   /* Spills may use MRFs 13-15 in the SIMD16 case.  Our texturing is done
> +    * using up to 11 MRFs starting from either m1 or m2, and fb writes
> can use
> +    * up to 13 (gen6+ simd16: 2 header + 8 color + 2 src0alpha + 2 omask)
> or
> +    * 15 (gen4-5 simd16: 2 header + 8 color + 1 aads + 2 src depth + 2 dst
> +    * depth), starting from m1.  If you're attentive, you may note that
> m1+15
> +    * = m16, which doesn't even exist.


There's a fencepost error in your reasoning here.  If fb writes start at m1
and use 15 registers, then they use registers m1 through m15, all of which
exist.

With the comment fixed, this patch is:

Reviewed-by: Paul Berry <stereotype441 at gmail.com>


>  In summary: We may not be able to
> +    * spill in SIMD16 mode, because we'd stomp the FB writes.
> +    */
> +   if (!spilled_any_registers) {
> +      bool mrf_used[BRW_MAX_MRF];
> +      get_used_mrfs(mrf_used);
> +
> +      for (int i = spill_base_mrf; i < BRW_MAX_MRF; i++) {
> +         if (mrf_used[i]) {
> +            fail("Register spilling not supported with m%d used", i);
> +          return;
> +         }
> +      }
> +
> +      spilled_any_registers = true;
> +   }
>
>     /* Generate spill/unspill instructions for the objects being
>      * spilled.  Right now, we spill or unspill the whole thing to a
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> index 71b4bf9..0e2dfd3 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> @@ -2722,6 +2722,8 @@ fs_visitor::fs_visitor(struct brw_context *brw,
>     this->force_uncompressed_stack = 0;
>     this->force_sechalf_stack = 0;
>
> +   this->spilled_any_registers = false;
> +
>     memset(&this->param_size, 0, sizeof(this->param_size));
>  }
>
> --
> 1.8.4.rc3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20131030/bd57c474/attachment.html>