[Mesa-dev] [Mesa-stable] [PATCH 7/8] i965: Assert that the scratch spaces are in range.

[Francisco Jerez]

Kenneth Graunke <kenneth at whitecape.org> writes:

> I don't know that anything actually guarantees this, but if we exceed
> the limits, we may end up overflowing and trashing random buffers that
> happen to be nearby in the VMA space, leading to rendering corruption,
> hangs, or worse.
>
> We should really fix this properly.  However, the pitfall has existed
> for ages, so for now we should at least detect it.
>
> Cc: "12.0" <mesa-stable at lists.freedesktop.org>
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/mesa/drivers/dri/i965/brw_fs.cpp | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index f1a1c87..104c20b 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -6001,7 +6001,21 @@ fs_visitor::allocate_registers(bool allow_spilling)
>            * size linearly with a range of [1kB, 12kB] and 1kB granularity.
>            */
>           prog_data->total_scratch = ALIGN(last_scratch, 1024);
> +
> +         assert(prog_data->total_scratch < 12 * 1024);

Looks like the following CTS and Piglit test cases hit this assertion
when run with INTEL_DEBUG=spill_fs:

  piglit.es31-cts.layout_binding.image2d_layout_binding_imageload_computeshader
  piglit.es31-cts.shader_storage_buffer_object.basic-stdlayout_ubo_ssbo-case2-cs
  piglit.spec.arb_compute_shader.linker.bug-93840

We can probably fix it later on, it will likely make things easier to
have a test case using more than 12 KB of scratch from the compute
shader.  Series is:

Reviewed-by: Francisco Jerez <currojerez at riseup.net>

>        }
> +
> +      /* We currently only support up to 2MB of scratch space.  If we
> +       * need to support more eventually, the documentation suggests
> +       * that we could allocate a larger buffer, and partition it out
> +       * ourselves.  We'd just have to undo the hardware's address
> +       * calculation by subtracting (FFTID * Per Thread Scratch Space)
> +       * and then add FFTID * (Larger Per Thread Scratch Space).
> +       *
> +       * See 3D-Media-GPGPU Engine > Media GPGPU Pipeline >
> +       * Thread Group Tracking > Local Memory/Scratch Space.
> +       */
> +      assert(prog_data->total_scratch < 2 * 1024 * 1024);
>     }
>  }
>  
> -- 
> 2.8.3
>
> _______________________________________________
> mesa-stable mailing list
> mesa-stable at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-stable
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160611/17cad0e3/attachment.sig>