Re: [RFA]: New function to supply only one x87 register

[Eli Zaretskii <eliz at delorie dot com>]

> Date: Fri, 9 Feb 2001 17:55:15 +0100 (MET)
> From: Mark Kettenis <kettenis@wins.uva.nl>
> 
>    I don't know enough about GDB's register cache to feel safe with such
>    assumptions.  What I'm trying to do is be consistent with the
>    documented API.  Since target.h's `to_fetch_registers' method says
>    "fetch register REGNO, or all regs if regno == -1", I'm trying to do
>    just that and nothing else.
> 
>    Isn't it dangerous to have functions in the infrastructure with
>    built-in implicit assumptions that are not documented anywhere, and
>    only hold because the current application-level code does what it
>    does?
> 
> Of course it is, but in this particular case it is wide-spread.  All
> systems with a SVR4-like /proc file system do this, as well as most
> systems that have a ptrace request that fetches a whole bunch of
> registers in one go.  By supplying all those registers at once, the
> cache becomes some sort of read-ahead cache.  This makes sense since
> GDB hardly ever needs only one register, and pre-fetching those saves
> a few system calls.  This probably doesn't matter much on go32, but I
> think you should do it anyway, at least for the FPU.

I can certainly do that (under a protest! ;-), but it simply seems
wrong.  I mean, let's take i386bsd-nat.c, for example:

    /* Fetch register REGNO from the inferior.  If REGNO is -1, do this
       for all registers (including the floating point registers).  */

    void
    fetch_inferior_registers (int regno)
    {
      gregset_t gregs;

      if (ptrace (PT_GETREGS, inferior_pid, (PTRACE_ARG3_TYPE) &gregs, 0) == -1)
	perror_with_name ("Couldn't get registers");

      supply_gregset (&gregs);

      if (regno == -1 || regno >= FP0_REGNUM)
	{
	  fpregset_t fpregs;

	  if (ptrace (PT_GETFPREGS, inferior_pid,
		      (PTRACE_ARG3_TYPE) &fpregs, 0) == -1)
	    perror_with_name ("Couldn't get floating point status");

	  supply_fpregset (&fpregs);
	}
    }

I remember staring at this for a few moments and thinking: this one
comlpetely ignores REGNO--isn't that a bug? and why does it call
ptrace for the general registers even if REGNO might be an FP
register--isn't _that_ a bug??

It simply doesn't feel right to do this sort of things, even if it is
somehow more efficient.  IMHO, a minor inefficiency is not worth
circumventing the established API, because it might cost much more in
maintenance down the road.

If we think it's a Good Thing to fetch all registers at once, let's
change the API, or let's have the target end cache them to be more
efficient (the DJGPP port already does that, btw).

> By supplying all those registers at once, the
> cache becomes some sort of read-ahead cache.  This makes sense since
> GDB hardly ever needs only one register, and pre-fetching those saves
> a few system calls.

Well, I thought about this, and I'm still not convinced there's a gain
here, at least not in all possible situations.

First, if GDB needs more than one FP register, it could just pass -1 as
REGNO and get them all.

Also, I can certainly think about a situation when GDB actually needs
only one FP register.  Imagine that I need to put a watchpoint on such
a register, for example, or display it at each stop.

> We probably should document this somewhere.

I'm afraid this aspect is so subtle that no matter where you document
it, it runs a high risk of being overlooked.

Btw, I found only two other platforms which currently use functions
from i387-nat.c, so it doesn't seem too late to make changes.