[Valgrind-developers] Re: CVS: valgrind/coregrind vg_dispatch.S,1.7,1.8 vg_from_ucode.c,1.18,1.19 vg_include.h,1.97,1.98 vg_scheduler.c,1.98,1.99 vg_transtab.c,1.12,1.13

[Jeremy F. <je...@go...>]

On Sat, 2002-11-30 at 07:09, Julian Seward wrote:

> 00-lazy-fp
> finally got round to doing this.  I added a FPU-save clause in the
> case for USESEG, which does a ccall and therefore is potentially 
> dangerous.

Great.  How about the multiply patch?

> 47-chained-bb   
> is finally in.  Thanks for that.  Required some minor mashing around
> to make the stats work out sensibly.  I've added counters for # chainings,
> # unchainings, and # of jumps via the dispatcher.  It gives a worst-case
> indirect count of about 16% for KDE apps.

Statically or dynamically?

> * you had VG_MAX_JUMPS set to 4; almost all bbs have 2 or less
>   jumps.  Is there a reason for having it at 4?  I changed it to 2.
>   Seems to work; it that OK ?  Saved 1 word per TCEntry compared with 4.

Initially it wouldn't work unless every BB had VG_MAX_JUMPS or fewer
jumpsites.  I added the sanity check to generate the fallback path later
on.

> * the number of dechain ops seems quite high .. did you implement
>   the optimisation you mentioned in which translations are only dechained if
>   they contain jumps to sector(s) which are about to be discarded?

Well, every jumpsite has an initial unchain, in order to establish the
call to VG_(patch_me), so those shouldn't be counted.  I've put up a
patch to fix this and the strangeness below; I'll try to check it in
myself (still haven't got around to making CVS work yet).

> 50-fast-cond
> 
> * This works (well).  However, at vg_from_ucode.c:1576 (rev 1.22) I had
>   to set      delta = 5+3+5+1 -1;  (ie decrease it by 1)
>   in order to avoid segfaults.  And the code emitted doesn't quite
>   correspond with the comment.  In particular the bit marked 
>   "$01					// 1" (what does this mean?)
>   is missing.
> 
>   Could you sanity-check this?  I don't quite understand what happened
>   here.  I'd like the comment and reality to match ... :)

Well, 50-fast-cond is relative to 48-chained-indirect; chained-indirect
adds a marker byte after the jumpsite so the unchaining machinery can
distinguish between direct and indirect jumps.  At first the markers
were the constants 1 or 2, but I later changed it to be 0x90 (nop) or
0x40 (inc %eip) so that they were proper 1-byte opcodes and would
confuse the decode logic (or me disassembling).

So, relative to 47-chained-bb, the delta should be 5+3+5 and the comment
shouldn't mention $01.

> Next on my hit list is 51-kill-inceip.

So you've decided to go with the SYNCEIP idea?

        J