[Mesa-dev] [PATCH] i965/fs: Fix FB writes that tried to use the non-existent m16 register.
eric at anholt.net
Tue Apr 24 16:13:28 PDT 2012
On Tue, 24 Apr 2012 14:09:13 -0700, Kenneth Graunke <kenneth at whitecape.org> wrote:
> A little analysis shows that the worst-case value for "nr" is 17:
> - base_mrf = 2 ... 2
> - header present (say gen == 5) ... 4
> - aa_dest_stencil_reg (stencil test) ... 5
> - SIMD16 mode: += 4 * reg_width ... 13
> - source_depth_to_render_target ... 15
> - dest_depth_reg ... 17
> This resulted in us setting base_mrf to 2 and mlen to 15. In other
> words, we'd try to use m2..m16. But m16 doesn't exist pre-Gen6. Also,
> the instruction scheduler data structures use arrays of size 16, so this
> would cause us to access them out of bounds.
> While the debugger system routine may need m0 and m1, we don't use it
> today, so the simplest solution is just to move base_mrf back to 1.
> That way, our worst case message fits in m1..m15, which is legal.
> An alternative would be to fail on SIMD16 in this case, but that seems
> a bit unfortunate if there's no real need to reserve m0 and m1.
> Fixes new piglit test shaders/depth-test-and-write on Ironlake.
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=48218
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
> Turns out the patch was correct, I just can't count :p
> With stencil testing enabled, we get an extra nr++ that I missed in my
> initial analysis. So the maximum message length is actually 15, not 14.
> 2 + 15 = 17 ==> we try to use m2..m16 ==> BOOM
There we go! These two are:
Reviewed-by: Eric Anholt <eric at anholt.net>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Size: 197 bytes
Desc: not available
More information about the mesa-dev