Re: [Valgrind-developers] Re: Another architectural thing ...

[Julian S. <js...@ac...>]

> Your idea is quite plausible.  Basically the skins would bypass the core's
> built-in way of handling stack updates in order to do them itself, because
> it can be more efficient that way.
>
> Which makes me think:  if we want to make this improvement for
> {Mem,Addr}check, any other skin that tracks %esp changes probably wants it
> as well.  So let's try to improve the general mechanism rather than
> implementing a {Mem,Addr}check-only optimisation.

Yes, indeed.  But your suggestion below is an improvement to the general
mechanism, not just to {Mem,Addr}check, yes?  


> You could then have a number of trackable events for skins to hook into:
>
>   new_mem_stack_aligned_4
>   new_mem_stack_aligned_8
>   etc.
>   die_mem_stack_aligned_4
>   die_mem_stack_aligned_8
>   etc.
>
> for the most common cases (eg. 4, 8, 12, 16, 20, 24).  They would be
> passed the old %esp.  The skins could have unrolled versions of the
> general stack-adjusting code for these cases.
>
> Also have:
>
>   new_mem_stack_aligned_gen
>   new_mem_stack
>   die_mem_stack_aligned_gen
>   die_mem_stack
>
> If a skin didn't provide these special case functions, the core could fall
> back to using the general case ones if they were provided -- this would be
> useful when first writing skins, when you don't want to write five
> different versions of the same function.  Ie. new_mem_stack_aligned_4
> would be used if present, but fall back to new_mem_stack_aligned_gen if
> present, but fall back to new_mem_stack if present, else do nothing.
>
> One complication -- how do we know at compile-time if a stack-adjustment
> is aligned?  We can't (AFAICT) so maybe the events shouldn't have any
> mention of alignment, and it's up to the skin to do an alignment check and
> speed up its actions based on this if it wants.  So the events might be
> new_mem_stack_4, new_mem_stack_8, ..., new_mem_stack_gen.

Yes.  Although in practice the stack is almost always kept at least 4-aligned,
we can't be sure of this, so all the functions need to check this.

Tell me if the following proposal makes sense: 

-------------
any skin which wishes to track stack
memory permissions must provide at least the general functions

   new_mem_stack
   die_mem_stack

They may optionally provide any subset of the following:

   {new,die}_mem_stack_{4,8,12,16,20,24,28,32}

in which case those will get called in preference, in cases where the code
generator can establish what the delta is, and generally when convenient for
the code generator.  Code generator will try to call the specialised fns but
does not promise to do so, if it isn't convenient for it.

The specialised functions are passed %ESP after the adjustment has been made,
ie the new value.  The general fns are passed old and new values.  None of the
functions can make any assumptions about alignment; they have to check for
themselves.
-------------

Another performance benefit, albeit a small one, is that the specialised
fns take one param rather than two, so that means less register shuffling
etc in the call sequence.

As a sanity check, let me ask (possibly again): does this scheme in any way
mess up the nice core/skin split, or generally clog up your architectural
cleanups in any other way?


> I'll definitely look into this once I've finished looking at moving
> malloc() et al out of core.

Great.

J