Re: CPU disassembler-memory accessor

[Ben Elliston <bje at redhat dot com>]

>>>>> "FChE" == Frank Ch Eigler <fche@redhat.com> writes:

  FChE> Yes, just one.  The read_disasm_memory_1 function is methinks
  FChE> overengineered.  It doesn't need to throw exceptions, nor handle
  FChE> accesses of other sizes.  Actually, why not just do a direct
  FChE> sid::bus-level read() on the accessor and do away with the
  FChE> read_disasm_memory* thingie altogether?

Thanks for the feedback.  How's this instead?

Index: include/sidcpuutil.h
===================================================================
RCS file: /cvs/cvsfiles/devo/sid/include/sidcpuutil.h,v
retrieving revision 1.28.2.4
diff -u -p -r1.28.2.4 sidcpuutil.h
--- include/sidcpuutil.h	2002/04/23 21:07:51	1.28.2.4
+++ include/sidcpuutil.h	2002/05/31 13:15:37
@@ -495,6 +495,7 @@ namespace sidutil
   protected:
     sid::bus* data_bus;
     sid::bus* insn_bus;
+    sid::bus* disassembler_bus;
 
   protected:
     template <typename BigOrLittleInt>
@@ -506,6 +507,8 @@ namespace sidutil
     template <typename BigOrLittleInt>
     BigOrLittleInt write_data_memory (sid::host_int_4 pc, sid::host_int_4 address, BigOrLittleInt value) const;
 
+    bool read_disasm_memory (sid::host_int_4 address, sid::big_int_1& value) const;
+
     // ------------------------------------------------------------------------
     
 public:
@@ -529,6 +532,8 @@ public:
 	add_accessor ("data-memory", & this->data_bus);
 	this->insn_bus = 0;
 	add_accessor ("insn-memory", & this->insn_bus);
+	this->disassembler_bus = 0;
+	add_accessor ("disassembler-memory", & this->disassembler_bus);
 	add_bus ("debugger-bus", & this->debugger_bus);
 
 	// pins
@@ -597,6 +602,23 @@ public:
       else
 	dynamic_cast <ofstream&> (s) << t;
       return s;
+    }
+
+    inline bool
+    basic_cpu::read_disasm_memory (sid::host_int_4 address, sid::big_int_1& value) const
+    {
+      sid::bus* bus;
+      bus = (this->disassembler_bus) ? this->disassembler_bus : this->insn_bus;
+      
+      try
+	{
+	  if (LIKELY (bus->read (address, value) == sid::bus::ok))
+	    return true;
+	}
+      catch (cpu_memory_fault& f)
+	{
+	  return false;
+	}
     }
   
     template <typename BigOrLittleInt>

Index: component/cgen-cpu/compCGEN.cxx
===================================================================
RCS file: /cvs/cvsfiles/devo/sid/component/cgen-cpu/compCGEN.cxx,v
retrieving revision 1.66.2.5
diff -u -p -r1.66.2.5 compCGEN.cxx
--- component/cgen-cpu/compCGEN.cxx	2002/04/03 18:43:36	1.66.2.5
+++ component/cgen-cpu/compCGEN.cxx	2002/05/31 13:15:35
@@ -184,10 +184,6 @@ cgen_bi_endian_cpu::cgen_read_memory(bfd
 {
   cgen_bi_endian_cpu *thisp = static_cast<cgen_bi_endian_cpu *>(info->application_data);
 
-  // We don't want to penalize the disassembler with memory latency counts, so we
-  // store it away here ...
-  host_int_8 prev_latency = thisp->total_latency;
-
   switch (length) {
 #if 0 // XXX not sure if this has byte order dependancies or not
   case 1:
@@ -204,13 +200,17 @@ cgen_bi_endian_cpu::cgen_read_memory(bfd
     break;
 #endif
   default:
-    for (int i = 0; i < length; i++)
-      *(myaddr + i) = thisp->read_insn_memory_1(0, memaddr + i);
+    {
+      big_int_1 value;
+      for (int i = 0; i < length; i++)
+	{
+	  if (! thisp->read_disasm_memory (memaddr + i, value))
+	    return 1;
+	  else
+	    *(myaddr + i) = value;
+	}
+    }
   }
-
-  // ... and restore it here.
-  thisp->total_latency = prev_latency;
-
   return 0;
 }
 
Index: component/cgen-cpu/common-xml/behavior.xml
===================================================================
RCS file: /cvs/cvsfiles/devo/sid/component/cgen-cpu/common-xml/behavior.xml,v
retrieving revision 1.1.10.1
diff -u -p -r1.1.10.1 behavior.xml
--- component/cgen-cpu/common-xml/behavior.xml	2002/03/14 01:04:39	1.1.10.1
+++ component/cgen-cpu/common-xml/behavior.xml	2002/05/31 13:15:35
@@ -32,9 +32,12 @@
       <p>Each instruction is first fetched from memory via the
       <accessor>insn-memory</accessor> accessor, and its decoding
       traced if the <attribute>trace-extract?</attribute> attribute is
-      set to a true value.  The decoded form may be cached
-      indefinitely afterwards, although this cache is flushed when the
-      <pin>flush-icache</pin> pin is driven.</p>
+      set to a true value.  To prevent unwanted cache side effects,
+      the <accessor>disassembler-memory</accessor> accessor can be
+      used and connected directly to main memory, bypassing any memory
+      caches.  The decoded form may be cached indefinitely afterwards,
+      although this cache is flushed when the <pin>flush-icache</pin>
+      pin is driven.</p>
 
       <p>The <attribute>engine-type</attribute> attribute specifies
       whether the "scache" ("semantic cache") or "pbb" ("pseudo basic
Index: component/cgen-cpu/common-xml/interface.xml
===================================================================
RCS file: /cvs/cvsfiles/devo/sid/component/cgen-cpu/common-xml/interface.xml,v
retrieving revision 1.2.10.1
diff -u -p -r1.2.10.1 interface.xml
--- component/cgen-cpu/common-xml/interface.xml	2002/03/14 01:04:39	1.2.10.1
+++ component/cgen-cpu/common-xml/interface.xml	2002/05/31 13:15:35
@@ -16,6 +16,7 @@
     <!-- accessors -->
     <defaccessor name="data-memory" accesses="any" behaviors="execution" />
     <defaccessor name="insn-memory" accesses="typically 4-byte accesses" behaviors="execution" />
+    <defaccessor name="disassembler-memory" accesses="any" behaviors="execution" />
 
 
     <!-- buses -->