RFC: Optimization to write_register_bytes()

Fernando Nasser fnasser@cygnus.com
Fri Jun 9 07:22:00 GMT 2000

Andrew Cagney wrote:
> Fernando Nasser wrote:
> >
> > We had a long thread on this that end up turning in what we are doing in
> > the future to improve the API etc.
> >
> > Somehow the down to Earth patch below was forgotten, and without this the
> > code to show MMX registers does not work.
> What happens if the code:
>       /* The register partially overlaps the range being written.  */
>       else
>         {
>           char regbuf[MAX_REGISTER_RAW_SIZE];
>           /* What's the overlap between this register's bytes and
>              those the caller wants to write?  */
>           int overlapstart = max (regstart, myregstart);
>           int overlapend   = min (regend,   myregend);
>           /* We may be doing a partial update of an invalid register.
>              Update it from the target before scribbling on it.  */
>           read_register_gen (regno, regbuf);
>           memcpy (registers + overlapstart,
>                   myaddr + (overlapstart - myregstart),
>                   overlapend - overlapstart);
>           target_store_registers (regno);
>         }
> is rewritten to use write_register_gen () instead of memcpy() to store
> the register?
> The function write_register_gen() only pushes through a register write
> if the register contents actually change.  The above code forces the
> register write regardless.
> While it isn't as optimal as bailing out of the loop early it definitly
> improves things and probably stops redundant writes and (besides, it
> makes the code far more readable :-).

Andrew, if you look carefully at the code of write_register_gen() you will
see that we will be using almost nothing of it except the test for no changes.
On the contrary, we would e adding overhead.  So, consider my patch with the
following added to write_register_bytes() on the segment above:

          /* If new value == old value, then don't bother doing the
             actual store. */
          if (memcmp (registers + overlapstart,
                  myaddr + (overlapstart - myregstart),
                  overlapend - overlapstart) == 0)

Now I am doing what you want but without adding any unnecessary tests.

I still would like to cut the loop short regardless.  We just don't need
to spend this time looping there. 

> --
> The second tweek I think may be safe is to keep track of how many bytes
> are still to be written.  If we just worry about the case where
> registers are contigious and assending then the required change should
> be easy.
>         if (we've just written out all the low bytes)
>            bump our lower bound and number of bytes to write;
>            if there are no more bytes to write
>              break;
> it does assume that the actual target doesn't need separate writes when
> two registers overlap.  I think that is safe as the original purpose of
> the function was to spray values such as structures across several
> registers.

This solution is cool as well, but the implementation in my patch is less
restringent.  I was wrong when I said that I required the registers to
be contiguous.  The only condition I add is that the last part comes last.
They can be non-contiguous and the initial pieces may even be out of order
(not that anyone should use that). 

I would stick to the "last piece written" test and fix the comments.

Fernando Nasser
Cygnus Solutions (a Red Hat company)    E-Mail:  fnasser@cygnus.com
2323 Yonge Street, Suite #300           Tel:  416-482-2661 ext. 311
Toronto, Ontario   M4P 2C9              Fax:  416-482-6299

More information about the Gdb-patches mailing list