On 20 December 2013 06:39, Topi Pohjolainen <topi.pohjolai...@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 <stereotype...@gmail.com> I've sent comments on patches 24, 27, 37, 38, 39, and 41. All the others are: Reviewed-by: Paul Berry <stereotype...@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.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
