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

Paul Berry stereotype441 at gmail.com
Mon Jan 20 20:35:44 PST 2014


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.

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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140120/8d3ba72b/attachment.html>


More information about the mesa-dev mailing list