[Mesa-dev] [PATCH] i965/fs: Fail the shader compile instead of asserting when we can't spill

[Jason Ekstrand]

On Thu, Sep 8, 2016 at 2:39 PM, Francisco Jerez <currojerez at riseup.net>
wrote:

> Jason Ekstrand <jason at jlekstrand.net> writes:
>
> > Blorp doesn't handle spilling so we set allow_spilling to false in that
> > case.  The blorp 16x MSAA resolve shader spills in 16-wide but not
> 8-wide.
> > This commit makes it so that we fail the 16-wide compile and successfully
> > fall back to 8-wide instead of just assert-failing when trying to compile
> > the 16-wide shader.
> >
> > Signed-off-by: Jason Ekstrand <jason at jlekstrand.net>
> > ---
> >  src/mesa/drivers/dri/i965/brw_fs.cpp | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> > index d0b55ae..73aa5d2 100644
> > --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> > @@ -5906,6 +5906,11 @@ fs_visitor::allocate_registers(bool
> allow_spilling)
> >     }
> >
> >     if (!allocated_without_spills) {
> > +      if (!allow_spilling) {
> > +         assert(dispatch_width > 8);
>
> Not sure it makes sense for the register allocator to set different
> requirements based on the build dispatch width, wouldn't it make sense
> to just fail() here consistently if allow_spilling is false regardless
> of the dispatch width, and let blorp assert fail if none of the dispatch
> widths it tries out compiles successfully?
>

Agreed.  I'll drop the assert.


> > +         fail("Failure to register allocate and spilling is not
> allowed.");
> > +      }
> > +
> >        /* We assume that any spilling is worse than just dropping back to
> >         * SIMD8.  There's probably actually some intermediate point where
> >         * SIMD16 with a couple of spills is still better.
> > @@ -5930,8 +5935,6 @@ fs_visitor::allocate_registers(bool
> allow_spilling)
> >        }
> >     }
> >
> > -   assert(last_scratch == 0 || allow_spilling);
> > -
> >     /* This must come after all optimization and register allocation,
> since
> >      * it inserts dead code that happens to have side effects, and it
> does
> >      * so based on the actual physical registers in use.
> > --
> > 2.5.0.400.gff86faf
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160908/f8bcda2c/attachment.html>