|
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
|