[Mesa-dev] [PATCH 0/5] Enable up to 24 MRF registers in gen6

[Iago Toral]

Hi Mark,

On Mon, 2015-09-21 at 17:45 -0700, Mark Janes wrote:
> Hi Iago,
> 
> According to my tests, this patch series fixes the gles2/gles3
> "functional.uniform_api.random.23" tests in dEQP, on sandybridge.
> 
> Do you see the same results?  Should this patch series be applied to the
> stable branch?

I can try to verify this, but I would not be surprised if that was the
case. This seems to fix a couple of bugs in bugzilla and it fixes a lot
of piglit tests in SNB when we force spilling, so if that test is
triggering spilling, there is a good chance that it had the same problem
and this patch fixed it. I'll try to confirm this today and let you
know.

As for whether this should go to stable, I suppose it depends on whether
we are confident that this won't break anything else... if you have not
seen any more issues with piglit/deqp on any other platform I guess
that's a good sign, but I'll let Ken make that call.

Iago

> thanks,
> 
> Mark
> 
> Iago Toral Quiroga <itoral at igalia.com> writes:
> 
> > There are some bug reports about shaders failing to compile in gen6
> > because MRF 14 is used when we need to spill. For example:
> > https://bugs.freedesktop.org/show_bug.cgi?id=86469
> > https://bugs.freedesktop.org/show_bug.cgi?id=90631
> >
> > Discussion in bugzilla pointed to the fact that gen6 might actually have
> > 24 MRF registers available instead of 16, so we could use other MRF
> > registers and avoid these conflicts (we still need to investigate why
> > some shaders need up to MRF 14 anyway, since this is not expected).
> >
> > Notice that the hardware docs are not clear about this fact:
> >
> > SNB PRM Vol4 Part2's "Table 5-4. MRF Registers Available in Device
> > Hardware" says "Number per Thread" - "24 registers"
> >
> > However, SNB PRM Vol4 Part1, 1.6.1 Message Register File (MRF) says:
> >
> > "Normal threads should construct their messages in m1..m15. (...)
> > Regardless of actual hardware implementation, the thread should
> > not assume th at MRF addresses above m15 wrap to legal MRF registers."
> >
> > Therefore experimentation was necessary to evaluate if we had these extra
> > MRF registers available or not. This was tested in gen6 using MRF
> > registers 21..23 for spilling and doing a full piglit run (all.py) forcing
> > spilling of everything on the FS backend. It was also tested by doing
> > spilling of everything on both the FS and the VS backends with a piglit run
> > of shader.py. In both cases no regressions were observed. In fact, many of
> > these tests where helped in the cases where we forced spilling, since that
> > triggered the same underlying problem described in the bug reports. Here are
> > some results using INTEL_DEBUG=spill_fs,spill_vec4 for a shader.py run on
> > gen6 hardware:
> >
> > Using MRFs 13..15 for spilling:
> > crash: 2, fail: 113, pass: 6621, skip: 5461
> >     
> > Using MRFs 21..23 for spilling:
> > crash: 2, fail: 12, pass: 6722, skip: 5461
> >
> > We might want to test this further with other instances of gen6 hardware
> > though... I am not sure that we can safely conclude that all implementations
> > of gen6 hardware have 24 MRF registers from my tests on just one particular
> > SandyBridge laptop.
> >
> > Iago Toral Quiroga (5):
> >   i965: Move MRF register asserts out of brw_reg.h
> >   i965: Turn BRW_MAX_MRF into a macro that accepts a hardware generation
> >   i965/fs: Use MRF registers 21-23 for spilling in gen6
> >   i965/vec4: Use MRF registers 21-23 for spilling in gen6
> >   i965: Maximum allowed size of SEND messages is 15 (4 bits)
> >
> >  src/mesa/drivers/dri/i965/brw_eu_emit.c            | 11 +++++----
> >  src/mesa/drivers/dri/i965/brw_fs.cpp               |  4 ++--
> >  src/mesa/drivers/dri/i965/brw_fs_generator.cpp     | 15 ++++++++----
> >  src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp  | 27 ++++++++++++----------
> >  src/mesa/drivers/dri/i965/brw_inst.h               |  3 +++
> >  src/mesa/drivers/dri/i965/brw_ir_vec4.h            |  2 +-
> >  src/mesa/drivers/dri/i965/brw_reg.h                |  9 ++++----
> >  .../drivers/dri/i965/brw_schedule_instructions.cpp |  4 ++--
> >  src/mesa/drivers/dri/i965/brw_vec4_generator.cpp   | 10 +++++---
> >  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp     | 15 +++++++-----
> >  10 files changed, 61 insertions(+), 39 deletions(-)
> >
> > -- 
> > 1.9.1
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>