From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eli Zaretskii To: kettenis@wins.uva.nl Cc: gdb-patches@sources.redhat.com Subject: Re: [RFA]: New function to supply only one x87 register Date: Sat, 10 Feb 2001 03:42:00 -0000 Message-id: <200102101142.GAA07322@indy.delorie.com> References: <200102081828.NAA25559@indy.delorie.com> <200102091459.JAA00914@indy.delorie.com> <200102091655.f19GtFK06000@debye.wins.uva.nl> X-SW-Source: 2001-02/msg00164.html > Date: Fri, 9 Feb 2001 17:55:15 +0100 (MET) > From: Mark Kettenis > > 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.