[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