From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30180 invoked by alias); 7 Sep 2011 14:12:21 -0000 Received: (qmail 30165 invoked by uid 22791); 7 Sep 2011 14:12:19 -0000 X-SWARE-Spam-Status: No, hits=1.0 required=5.0 tests=AWL,BAYES_00,BOTNET,RDNS_NONE,TW_EG X-Spam-Check-By: sourceware.org Received: from Unknown (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 07 Sep 2011 14:12:05 +0000 Received: (qmail 29584 invoked from network); 7 Sep 2011 14:12:04 -0000 Received: from unknown (HELO scottsdale.localnet) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 7 Sep 2011 14:12:04 -0000 From: Pedro Alves To: Doug Evans Subject: Re: PR cli/13110 Date: Wed, 07 Sep 2011 14:28:00 -0000 User-Agent: KMail/1.13.6 (Linux/2.6.38-11-generic; KDE/4.7.0; x86_64; ; ) Cc: gdb-patches@sourceware.org References: <201109051656.53563.pedro@codesourcery.com> In-Reply-To: MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201109071512.01995.pedro@codesourcery.com> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-09/txt/msg00123.txt.bz2 On Wednesday 07 September 2011 01:45:36, Doug Evans wrote: > On Mon, Sep 5, 2011 at 8:56 AM, Pedro Alves wrote: > > > > > > 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 gdb/ * infrun.c (prepare_for_detach, wait_for_inferior) (fetch_inferior_event): Don't flush the register cache. * remote.c (struct stop_reply) : 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;