|
From: Florian K. <br...@ac...> - 2013-01-27 21:57:59
Attachments:
valgrind-ite-patch
vex-ite-patch
|
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. Basically, IRExpr_Mux0X(cond, iffalse, iftrue) gets replaced with IRExpr_ITE(cond, iftrue, iffalse). Although this is a mechanical change, swapping the arguments in an automated way (perhaps with emacs macros or so), was beyond my emacs foo. So there is a fair amount of hand work and, thusly, needs review. I'll wait for that. I've tried to avoid reformatting the context code as much as possible but for some instances in the mips port that was just impossible.. Florian |
|
From: Дмитрий Д. <di...@gm...> - 2013-01-28 10:13:18
|
may be smth like http://lwn.net/Articles/315686/ Semantic patching with Coccinelle can help? Thanks, Dmitry 2013/1/28 Florian Krohm <br...@ac...>: > 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. > > Basically, IRExpr_Mux0X(cond, iffalse, iftrue) gets replaced with > IRExpr_ITE(cond, iftrue, iffalse). Although this is a mechanical change, > swapping the arguments in an automated way (perhaps with emacs macros or > so), was beyond my emacs foo. So there is a fair amount of hand work > and, thusly, needs review. I'll wait for that. > I've tried to avoid reformatting the context code as much as possible > but for some instances in the mips port that was just impossible.. > > Florian > > ------------------------------------------------------------------------------ > Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS, > MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current > with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft > MVPs and experts. ON SALE this month only -- learn more at: > http://p.sf.net/sfu/learnnow-d2d > _______________________________________________ > Valgrind-developers mailing list > Val...@li... > https://lists.sourceforge.net/lists/listinfo/valgrind-developers > |
|
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 |
|
From: Josef W. <Jos...@gm...> - 2013-01-28 17:08:54
|
Am 28.01.2013 17:32, schrieb Julian Seward: > 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. Curious: what is GSL? Josef |
|
From: Julian S. <js...@ac...> - 2013-01-28 17:27:55
|
On 01/28/2013 06:06 PM, Josef Weidendorfer wrote: > Curious: what is GSL? http://en.wikipedia.org/wiki/Gnu_Scientific_Library A big library of floating point routines, with an excellent and huge test suite, that is really useful for checking V's FP (and integer) insn set simulation. It has found bugs that none of the in-tree insn set tests have. J |
|
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
|
|
From: Julian S. <js...@ac...> - 2013-01-30 08:36:26
|
>> 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. Looks good for x86, amd64, arm, at least. >> in some way, by comparing generated code size before/after for (eg) >> perf/bz2 and perf/ffbench. > Sure, here you go. Basically identical: Good, thanks. >> 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. Yup. ---- So, with all that in place it's possible to get on and explain fun facts to iropt, like ITE(c,x,ITE(!c,y,z)) -> ITE(c,x,y), etc. J |