[Mesa-dev] [PATCH 42/42] i965/blorp: switch eu-emitter to use FS IR and fs_generator

Pohjolainen, Topi topi.pohjolainen at intel.com
Mon Jan 20 22:27:07 PST 2014


On Mon, Jan 20, 2014 at 08:35:44PM -0800, Paul Berry wrote:
>    On 20 December 2013 06:39, Topi Pohjolainen <topi.pohjolainen at intel.com>
>    wrote:
> 
>      Unfortunately the unit tests need to be patched as well. This is
>      because the direct eu-emitter only patches jump counters for
>      if-else (see patch_IF_ELSE()) while the fs_generator patches the
>      endif as well (see brw_set_uip_jip()).
> 
>    I think the fact that blorp wasn't patching jump counters for if-else was
>    a previously undiscovered bug in blorp.  I suspect that the reason we
>    never discovered it was because blorp doesn't contain complex enough flow
>    control to cause an endif to ever execute a jump.
> 
>    I'd recommend splitting this patch into two patches:
> 
>    1. fix the previously undiscovered blorp bug (by having blorp call
>    brw_set_uip_jip() at the appropriate time) and update the unit tests.

I'll work on this first, and then rebase the rest on top.

> 
>    2. switch eu-emitter to use FS IR and fs_generator.
> 
>    That will make it a lot easier to reassure ourselves that the changes are
>    correct, and in the unlikely event that we later discover that they
>    aren't, it'll make it easier to bisect to the problem.
> 
>    With this patch split, it is:
> 
>    Reviewed-by: Paul Berry <stereotype441 at gmail.com>
>    I've sent comments on patches 24, 27, 37, 38, 39, and 41.  All the others
>    are:
> 
>    Reviewed-by: Paul Berry <stereotype441 at gmail.com>
> 
>    I really like how you structured this series, Topi.  It was easy to follow
>    what you did and to reassure myself that it was correct.  And I'm really
>    glad you did it, since it led us to discover two preexisting bugs!  Nice
>    work.

Well, I'm really glad you liked it. I desperately wanted to avoid a mega patch
but the same time would have liked to do the changes in place instead of the
split into a separate emitter. After a while I just gave up as I couldn't get
anywhere and opted for the split instead.


More information about the mesa-dev mailing list