Re: [Valgrind-developers] allowing skins to instrument conditional jumps

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

On Wed, 2002-10-02 at 12:25, Nicholas Nethercote wrote:
    On 2 Oct 2002, Jeremy Fitzhardinge wrote:
    
    > At present I'm using a single global, which means that I'll be creating
    > spurious edges when there's context switches between threads.  The
    > obvious place to store the information is in the baseBlock, and have it
    > copied to/from the thread state on context switch.  I didn't see a
    > mechanism for allocating variable space in the baseBlock, nor a way of
    > conveniently addressing baseBlock offsets directly.  Should I add it?
    > Or some other way of storing per-thread information?
    
    Cachegrind stores variable-sized basic-block information.  It is pretty
    low-level and dirty:  it allocates a flat array in which cost centres of
    different sizes are all packed in together, with different cost centre
    types distinguished by a tag.  The basic blocks' arrays are stored in a
    hash table.

Yes, I've got that.  I have a hash which keeps per-basic-block
information.  But what I also want it a hash which keeps a count of
control flow edges between basic blocks.  That is, the key of the hash
is not orig_eip, but the tuple (from_bb, to_bb).  The way I maintain
this is by inserting an assignment to a global variable "prev_bb" (ie,
code to do prev_bb = cur_eip) just before each JMP instruction
(conditional or otherwise).  Then, at the start of each basic block, I
update the edge count structure by looking up (and possibly creating)
the tuple (prev_bb, cur_eip).

The trouble with this scheme is that if the dispatch loop decides that
it is time to switch threads, prev_bb will have been set by the previous
thread, and therefore the control flow graph will have spurious edges
which represent context switches.  While this isn't completely
undesirable, it isn't what I want to measure at the moment.

To solve this, prev_bb needs to be a per-thread value rather than a
global one.  It seems to me that a clean way of solving this is to
introduce a mechanism which is analogous to VG_(register_*_helper) which
allows a skin to allocate space in the baseBlock, with a change to the
scheduler to save and restore the values on context switch and some way
to generate uInstr code to load and store them.

	J