Re: [PATCH 2/3] skip_prolgoue (amd64)

[Yao Qi <yao at codesourcery dot com>]

On 12/08/2013 04:25 PM, Doug Evans wrote:
> If you're going for correctness here, note that this is just another heuristic.
> It gets one closer, but as long as at least one thread is still running ....
> 

I remind myself adding comments like that, but forgot doing so in the
end.  Add it in the comments in the updated patch.

> Question: Does this obviate the need to call target_dcache_invalidate elsewhere?
> If so, removing other uses that this subsumes should be part of this patch.
> 

Presently, target_dcache_invalidate is called by some places:

 - In commands on dcache and memattr,
 - In target_load and target_resume,
 - In tfind_1,
 - In prepare_execute_command,

I am wondering we may remove the call in prepare_execute_command,
because it is more "precise" to flush cache for every event than for
every command.  However, I am not sure, but I attach the patch.

> Nit: "cache would be staled" is bad English.
> "the cache could be stale" would be better, though I'm sure there is
> something even better than that.
> [I suspect there's at least one too many commas in there too, but one
> can nitpick too much. :-)]

Fixed.

-- 
Yao (éå)

gdb:

2013-12-09  Yao Qi  <yao@codesourcery.com>

	* infrun.c: Include "target-dcache.h".
	(handle_inferior_event): Flush target cache.
---
 gdb/infrun.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 3b55583..2901d09 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -60,6 +60,7 @@
 #include "objfiles.h"
 #include "completer.h"
 #include "target-descriptions.h"
+#include "target-dcache.h"
 
 /* Prototypes for local functions */
 
@@ -3168,6 +3169,12 @@ handle_inferior_event (struct execution_control_state *ecs)
 {
   enum stop_kind stop_soon;
 
+  /* If we've got an event from target, it means the target was
+     running and cache could be stale.  Flush target cache before
+     handling each event.  This is just a heuristic.  Running threads
+     may modify target memory, but we don't get any event.  */
+  target_dcache_invalidate ();
+
   if (ecs->ws.kind == TARGET_WAITKIND_IGNORE)
     {
       /* We had an event in the inferior, but we are not interested in
-- 
1.7.7.6

gdb:

2013-12-09  Yao Qi  <yao@codesourcery.com>

	* top.c: Don't include "target-dcache.h".
	(prepare_execute_command): Remove the call to
	target_dcache_invalidate.
---
 gdb/top.c |    8 --------
 1 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/gdb/top.c b/gdb/top.c
index 8ce1a9f..b7df81b 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -28,7 +28,6 @@
 #include "exceptions.h"
 #include <signal.h>
 #include "target.h"
-#include "target-dcache.h"
 #include "breakpoint.h"
 #include "gdbtypes.h"
 #include "expression.h"
@@ -332,13 +331,6 @@ prepare_execute_command (void)
   mark = value_mark ();
   cleanup = make_cleanup_value_free_to_mark (mark);
 
-  /* With multiple threads running while the one we're examining is
-     stopped, the dcache can get stale without us being able to detect
-     it.  For the duration of the command, though, use the dcache to
-     help things like backtrace.  */
-  if (non_stop)
-    target_dcache_invalidate ();
-
   return cleanup;
 }
 
-- 
1.7.7.6