|
From: Julian S. <js...@ac...> - 2013-01-28 16:33:06
|
On 01/27/2013 10:57 PM, Florian Krohm wrote: > Attached are two patches for valgrind and VEX to make the change from > Iex_Mux0X to Iex_ITE. I've regtested this on x86-64 (64-bit only), s390, > and ppc64. No new regressions. arm and mips are completely untested. That was quick! It looks OK to me, on first scan. I suggest to commit it, as we have pretty good insn set tests, and I will incrementally verify it using GSL over the next week or so. That should shake out any badness. I'm inclined to re-read the Memcheck stuff once it's landed as it will be easier to see the context in-place. Pre landing .. I was a bit concerned about the AvailExpr cases for (ex-) Mux0X in ir_opt.c. Did the Mxxx cases have their 2nd and 3rd args swapped? Also, maybe rename Mxxx to Ixxx or something. It kind of looks like there was no swapping because there are both M?ct and M?tc and so any expression that was representable before as an AvailExpr is still representable now. But I'm not 100% sure that the impedance-match in/out code is correct. FWIW, I didn't stare at it long enough to be sure either way. Also it would be nice to have a quick check that iropt isn't borked in some way, by comparing generated code size before/after for (eg) perf/bz2 and perf/ffbench. J |