On Mon, Jan 20, 2014 at 08:35:44PM -0800, Paul Berry wrote: > 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.
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 <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. 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. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
