From: Florian K. <br...@ac...> - 2013-01-29 03:51:34
|
On 01/28/2013 11:32 AM, Julian Seward wrote: > 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. OK I will commit soonish (today). > 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? Ah, I see.... You're referring to, e.g. this here: - /* Mux0X(tmp,const,tmp) */ + /* ITE(tmp,const,tmp) */ struct { IRTemp co; IRConst con0; IRTemp eX; } Mtct; That should read: ITE(tmp,tmp,const) So, yes the comment was wrong and misleading. > Also, maybe rename Mxxx to Ixxx or something. Yes, definitely. I left it this way on purpose, because it minimises the changes in that area. So, a post patch should clean this up. Also, eX -> e1 seems appropriate etc. > 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. I think it's OK, because I converted ir_opt.c twice. The first time I screwed it up the AvailExpr stuff leading to infinite loops and miscomparing tests :) > > 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. Sure, here you go. Basically identical: BEFORE conversion bigcode1 transtab: new 30,048 (301,770 -> 6,242,784; ratio 206:10) bigcode2 transtab: new 114,422 (1,077,942 -> 22,899,680; ratio 212:10) bz2 transtab: new 3,699 (99,494 -> 1,554,709; ratio 156:10) fbench transtab: new 2,316 (54,725 -> 868,347; ratio 158:10) ffbench transtab: new 2,116 (52,001 -> 826,031; ratio 158:10) heap transtab: new 1,836 (40,759 -> 659,259; ratio 161:10) heap-pdb4 transtab: new 1,865 (41,287 -> 668,163; ratio 161:10) many-loss-records transtab: new 1,959 (43,806 -> 707,036; ratio 161:10) many-xpts transtab: new 1,803 (39,703 -> 645,026; ratio 162:10) sarp transtab: new 1,715 (38,525 -> 619,333; ratio 160:10) tinycc transtab: new 2,323 (54,156 -> 867,851; ratio 160:10) AFTER conversion bigcode1 transtab: new 30,048 (301,770 -> 6,242,791; ratio 206:10) bigcode2 transtab: new 114,422 (1,077,937 -> 22,899,687; ratio 212:10) bz2 transtab: new 3,699 (99,489 -> 1,554,709; ratio 156:10) fbench transtab: new 2,316 (54,725 -> 868,347; ratio 158:10) ffbench transtab: new 2,116 (52,011 -> 826,031; ratio 158:10) heap transtab: new 1,836 (40,754 -> 659,259; ratio 161:10) heap-pdb4 transtab: new 1,865 (41,277 -> 668,163; ratio 161:10) many-loss-records transtab: new 1,959 (43,801 -> 707,036; ratio 161:10) many-xpts transtab: new 1,803 (39,708 -> 645,019; ratio 162:10) sarp transtab: new 1,715 (38,525 -> 619,333; ratio 160:10) tinycc transtab: new 2,323 (54,156 -> 867,851; ratio 160:10) Florian |