* PR cli/13110
@ 2011-09-05 16:50 Pedro Alves
2011-09-07 1:29 ` Doug Evans
0 siblings, 1 reply; 5+ messages in thread
From: Pedro Alves @ 2011-09-05 16:50 UTC (permalink / raw)
To: gdb-patches
<http://sourceware.org/ml/gdb/2011-09/msg00019.html>
Tested on x86_64-linux and applied.
Repeating what I said on gdb@, I'm now wondering if we actually ever
need the registers_changed call here or in wait_for_inferior
nowadays --- we flush threads' register caches when we resume
them (target_resume).
--
Pedro Alves
2011-09-05 Pedro Alves <pedro@codesourcery.com>
PR cli/13110
gdb/
* infrun.c (fetch_inferior_event): Check if there's a selected
thread before checking if the selected thread is executing.
---
gdb/infrun.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
Index: src/gdb/infrun.c
===================================================================
--- src.orig/gdb/infrun.c 2011-09-05 15:50:49.000000000 +0100
+++ src/gdb/infrun.c 2011-09-05 15:57:10.693964411 +0100
@@ -2749,7 +2749,9 @@ fetch_inferior_event (void *client_data)
switches threads anyway). If we didn't do this, a spurious
delayed event in all-stop mode would make the user lose the
selected frame. */
- if (non_stop || is_executing (inferior_ptid))
+ if (non_stop
+ || (!ptid_equal (inferior_ptid, null_ptid)
+ && is_executing (inferior_ptid)))
registers_changed ();
make_cleanup_restore_integer (&execution_direction);
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: PR cli/13110
2011-09-05 16:50 PR cli/13110 Pedro Alves
@ 2011-09-07 1:29 ` Doug Evans
2011-09-07 14:28 ` Pedro Alves
0 siblings, 1 reply; 5+ messages in thread
From: Doug Evans @ 2011-09-07 1:29 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On Mon, Sep 5, 2011 at 8:56 AM, Pedro Alves <pedro@codesourcery.com> wrote:
> <http://sourceware.org/ml/gdb/2011-09/msg00019.html>
>
> Tested on x86_64-linux and applied.
>
> Repeating what I said on gdb@, I'm now wondering if we actually ever
> need the registers_changed call here or in wait_for_inferior
> nowadays --- we flush threads' register caches when we resume
> them (target_resume).
>
> --
> Pedro Alves
>
> 2011-09-05 Pedro Alves <pedro@codesourcery.com>
>
> PR cli/13110
>
> gdb/
> * infrun.c (fetch_inferior_event): Check if there's a selected
> thread before checking if the selected thread is executing.
>
> ---
> gdb/infrun.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> Index: src/gdb/infrun.c
> ===================================================================
> --- src.orig/gdb/infrun.c 2011-09-05 15:50:49.000000000 +0100
> +++ src/gdb/infrun.c 2011-09-05 15:57:10.693964411 +0100
> @@ -2749,7 +2749,9 @@ fetch_inferior_event (void *client_data)
> switches threads anyway). If we didn't do this, a spurious
> delayed event in all-stop mode would make the user lose the
> selected frame. */
> - if (non_stop || is_executing (inferior_ptid))
> + if (non_stop
> + || (!ptid_equal (inferior_ptid, null_ptid)
> + && is_executing (inferior_ptid)))
> registers_changed ();
>
> make_cleanup_restore_integer (&execution_direction);
>
The code may be to additionally check if there's a selected thread,
but when I read it I think "When is null_ptid ever executing?".
Comment in the code is required IMO.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: PR cli/13110
2011-09-07 1:29 ` Doug Evans
@ 2011-09-07 14:28 ` Pedro Alves
2011-09-07 17:42 ` Doug Evans
0 siblings, 1 reply; 5+ messages in thread
From: Pedro Alves @ 2011-09-07 14:28 UTC (permalink / raw)
To: Doug Evans; +Cc: gdb-patches
On Wednesday 07 September 2011 01:45:36, Doug Evans wrote:
> On Mon, Sep 5, 2011 at 8:56 AM, Pedro Alves <pedro@codesourcery.com> wrote:
> > <http://sourceware.org/ml/gdb/2011-09/msg00019.html>
> >
> > Tested on x86_64-linux and applied.
> >
> > Repeating what I said on gdb@, I'm now wondering if we actually ever
> > need the registers_changed call here or in wait_for_inferior
> > nowadays --- we flush threads' register caches when we resume
> > them (target_resume).
> > Index: src/gdb/infrun.c
> > ===================================================================
> > --- src.orig/gdb/infrun.c 2011-09-05 15:50:49.000000000 +0100
> > +++ src/gdb/infrun.c 2011-09-05 15:57:10.693964411 +0100
> > @@ -2749,7 +2749,9 @@ fetch_inferior_event (void *client_data)
> > switches threads anyway). If we didn't do this, a spurious
> > delayed event in all-stop mode would make the user lose the
> > selected frame. */
> > - if (non_stop || is_executing (inferior_ptid))
> > + if (non_stop
> > + || (!ptid_equal (inferior_ptid, null_ptid)
> > + && is_executing (inferior_ptid)))
> > registers_changed ();
> >
> > make_cleanup_restore_integer (&execution_direction);
> >
>
> The code may be to additionally check if there's a selected thread,
> but when I read it I think "When is null_ptid ever executing?".
>
> Comment in the code is required IMO.
How about removing the registers_changed calls once and
for all?
Tested on x86_64-linux native|gdbserver x sync|async.
--
Pedro Alves
2011-09-07 Pedro Alves <pedro@codesourcery.com>
gdb/
* infrun.c (prepare_for_detach, wait_for_inferior)
(fetch_inferior_event): Don't flush the register cache.
* remote.c (struct stop_reply) <regcache>: Add comment.
---
gdb/infrun.c | 33 ---------------------------------
gdb/remote.c | 4 ++++
2 files changed, 4 insertions(+), 33 deletions(-)
Index: src/gdb/infrun.c
===================================================================
--- src.orig/gdb/infrun.c 2011-09-07 13:21:46.877976256 +0100
+++ src/gdb/infrun.c 2011-09-07 14:41:52.954338077 +0100
@@ -2580,14 +2580,6 @@ prepare_for_detach (void)
overlay_cache_invalid = 1;
- /* We have to invalidate the registers BEFORE calling
- target_wait because they can be loaded from the target while
- in target_wait. This makes remote debugging a bit more
- efficient for those targets that provide critical registers
- as part of their normal status mechanism. */
-
- registers_changed ();
-
if (deprecated_target_wait_hook)
ecs->ptid = deprecated_target_wait_hook (pid_ptid, &ecs->ws, 0);
else
@@ -2657,14 +2649,7 @@ wait_for_inferior (void)
{
struct cleanup *old_chain;
- /* We have to invalidate the registers BEFORE calling target_wait
- because they can be loaded from the target while in target_wait.
- This makes remote debugging a bit more efficient for those
- targets that provide critical registers as part of their normal
- status mechanism. */
-
overlay_cache_invalid = 1;
- registers_changed ();
if (deprecated_target_wait_hook)
ecs->ptid = deprecated_target_wait_hook (waiton_ptid, &ecs->ws, 0);
@@ -2734,26 +2719,8 @@ fetch_inferior_event (void *client_data)
running any breakpoint commands. */
make_cleanup_restore_current_thread ();
- /* We have to invalidate the registers BEFORE calling target_wait
- because they can be loaded from the target while in target_wait.
- This makes remote debugging a bit more efficient for those
- targets that provide critical registers as part of their normal
- status mechanism. */
-
overlay_cache_invalid = 1;
- /* But don't do it if the current thread is already stopped (hence
- this is either a delayed event that will result in
- TARGET_WAITKIND_IGNORE, or it's an event for another thread (and
- we always clear the register and frame caches when the user
- switches threads anyway). If we didn't do this, a spurious
- delayed event in all-stop mode would make the user lose the
- selected frame. */
- if (non_stop
- || (!ptid_equal (inferior_ptid, null_ptid)
- && is_executing (inferior_ptid)))
- registers_changed ();
-
make_cleanup_restore_integer (&execution_direction);
execution_direction = target_execution_direction ();
Index: src/gdb/remote.c
===================================================================
--- src.orig/gdb/remote.c 2011-09-07 13:21:46.877976256 +0100
+++ src/gdb/remote.c 2011-09-07 14:41:52.954338077 +0100
@@ -4898,6 +4898,10 @@ struct stop_reply
struct target_waitstatus ws;
+ /* Expedited registers. This makes remote debugging a bit more
+ efficient for those targets that provide critical registers as
+ part of their normal status mechanism (as another roundtrip to
+ fetch them is avoided). */
VEC(cached_reg_t) *regcache;
int stopped_by_watchpoint_p;
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: PR cli/13110
2011-09-07 14:28 ` Pedro Alves
@ 2011-09-07 17:42 ` Doug Evans
2011-09-14 13:40 ` Pedro Alves
0 siblings, 1 reply; 5+ messages in thread
From: Doug Evans @ 2011-09-07 17:42 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On Wed, Sep 7, 2011 at 7:12 AM, Pedro Alves <pedro@codesourcery.com> wrote:
> How about removing the registers_changed calls once and
> for all?
Having/requiring them to be littered throughout the code always seemed
a tad fragile ...
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: PR cli/13110
2011-09-07 17:42 ` Doug Evans
@ 2011-09-14 13:40 ` Pedro Alves
0 siblings, 0 replies; 5+ messages in thread
From: Pedro Alves @ 2011-09-14 13:40 UTC (permalink / raw)
To: Doug Evans; +Cc: gdb-patches
On Wednesday 07 September 2011 15:12:01, Pedro Alves wrote:
> 2011-09-07 Pedro Alves <pedro@codesourcery.com>
>
> gdb/
> * infrun.c (prepare_for_detach, wait_for_inferior)
> (fetch_inferior_event): Don't flush the register cache.
> * remote.c (struct stop_reply) <regcache>: Add comment.
>
Now applied, thanks.
--
Pedro Alves
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-09-14 12:32 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-05 16:50 PR cli/13110 Pedro Alves
2011-09-07 1:29 ` Doug Evans
2011-09-07 14:28 ` Pedro Alves
2011-09-07 17:42 ` Doug Evans
2011-09-14 13:40 ` Pedro Alves
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox