PATCH: replace cpu_trace_stream subclass with rdbuf swap

PATCH: replace cpu_trace_stream subclass with rdbuf swap

[Jim Blandy]

Below is a patch against the current SID sources to make
sidutil::basic_cpu::trace_stream into an ordinary std::ostream object,
which fixes the bug described in my earlier message, but does so by
removing hair from sidcputil.h (and a bit elsewhere), instead of
adding to it.

It builds, but I haven't tested it.  The original patch was developed
on an internal Red Hat branch and tested against an internal Red Hat
cpu.

The full SID takes an awful long time to build, and I'm not sure what
toolchain and BSP to match it against.  Do folks here have any advice
on how to pare down the SID build and which architecture to target to
test this with as little fuss as possible?


sid/include/ChangeLog:
2004-01-21  Jim Blandy  <jimb@redhat.com>

	* sidcpuutil.h (sidutil::basic_cpu::trace_stream): Make this an
	ordinary std::ostream.
	(sidutil::basic_cpu::update_trace_destination): Instead of
	setting/clearing the cout_p flag in our subclassed stream, just
	call std::ostream::rdbuf to change trace_stream's underlying
	streambuf to cout's, or to a std::filebuf for the trace file.
	(sidutil::basic_cpu::basic_cpu): Initialize trace_stream to send
	its output to cout's rdbuf (standard output).
        (sidutil::basic_cpu::cpu_trace_stream): Delete class.
	(operator<<(sidutil::basic_cpu::cpu_trace_stream): Delete.

sid/component/cgen-cpu/ChangeLog:
2004-01-21  Jim Blandy  <jimb@redhat.com>

	* compCGEN.cxx (end_trace): Send 'endl' to trace_stream, rather
	than calling 'end_line'.  trace_stream is an ordinary std::ostream
	now, so things should work.

Index: sid/component/cgen-cpu/compCGEN.cxx
===================================================================
RCS file: /cvs/src/src/sid/component/cgen-cpu/compCGEN.cxx,v
retrieving revision 1.12
diff -c -r1.12 compCGEN.cxx
*** sid/component/cgen-cpu/compCGEN.cxx	29 Aug 2003 19:26:22 -0000	1.12
--- sid/component/cgen-cpu/compCGEN.cxx	22 Jan 2004 05:42:08 -0000
***************
*** 250,256 ****
  void
  cgen_bi_endian_cpu::end_trace ()
  {
!   trace_stream.end_line ();
  }
  
  // Counter support
--- 250,256 ----
  void
  cgen_bi_endian_cpu::end_trace ()
  {
!   trace_stream << endl;
  }
  
  // Counter support
Index: sid/include/sidcpuutil.h
===================================================================
RCS file: /cvs/src/src/sid/include/sidcpuutil.h,v
retrieving revision 1.27
diff -c -r1.27 sidcpuutil.h
*** sid/include/sidcpuutil.h	21 Oct 2003 21:38:24 -0000	1.27
--- sid/include/sidcpuutil.h	22 Jan 2004 05:42:10 -0000
***************
*** 213,248 ****
      // tracing
    private:
      string trace_filename;
-     class cpu_trace_stream: public std::ofstream
-     {
-     public:
-       cpu_trace_stream ()
- 	:cout_p (true) {}
-       cpu_trace_stream (const std::string& filename)
- 	:std::ofstream (filename.c_str ()), cout_p (false) {}
-       void divert_to_file () { cout_p = false; }
-       void divert_to_cout () { cout_p = true; }
-       void open (const std::string& filename)
-       {
- 	std::ofstream::open (filename.c_str (), std::ios::app);
- 	cout_p = false;
-       }
-       void end_line ()
-       {
- 	if (LIKELY (cout_p))
- 	  std::cout << std::endl;
- 	else
- 	  *this << std::endl;
-       }
-     private:
-       bool cout_p;
- 
-       template <typename T> friend
-       basic_cpu::cpu_trace_stream& operator<< (basic_cpu::cpu_trace_stream& s, T t);
-     };
- 
-     template <typename T> friend
-     cpu_trace_stream& operator<< (cpu_trace_stream& s, T t);
  
    public:
      bool trace_extract_p;
--- 213,218 ----
***************
*** 252,258 ****
      bool trace_counter_p;
      bool final_insn_count_p;
      bool enable_step_trap_p;
!     cpu_trace_stream trace_stream;
  
      void step_pin_handler (sid::host_int_4)
        {
--- 222,228 ----
      bool trace_counter_p;
      bool final_insn_count_p;
      bool enable_step_trap_p;
!     std::ostream trace_stream;
  
      void step_pin_handler (sid::host_int_4)
        {
***************
*** 353,370 ****
      void
      update_trace_destination ()
      {
        if (trace_filename == "-")
! 	trace_stream.divert_to_cout ();
        else
  	{
! 	  trace_stream.close ();
! 	  trace_stream.open (trace_filename);
  	  trace_stream << "start of trace" << std::endl;
! 	  if (trace_stream.good ())
! 	    trace_stream.divert_to_file ();
! 	  else
  	    trace_filename = "io-error!";
! 	}
      }
  
      // Infer a change to trace_result_p after one of the other general 
--- 323,349 ----
      void
      update_trace_destination ()
      {
+       // Save trace_stream's original read buffer, so we can free it
+       // once we've installed the new one.
+       std::streambuf *old_buf = trace_stream.rdbuf ();
+ 
        if (trace_filename == "-")
!         trace_stream.rdbuf (std::cout.rdbuf ());
        else
  	{
!           std::filebuf *new_buf = new std::filebuf ();
! 
!           new_buf->open (trace_filename.c_str (), 
!                          std::ios::app | std::ios::out);
!           trace_stream.rdbuf (new_buf);
  	  trace_stream << "start of trace" << std::endl;
! 	  if (! trace_stream.good ())
  	    trace_filename = "io-error!";
!         }
! 
!       // Close and free the old buf, unless we're sharing it with std::cout.
!       if (old_buf != std::cout.rdbuf ())
!         delete (old_buf);
      }
  
      // Infer a change to trace_result_p after one of the other general 
***************
*** 545,551 ****
        pc_set_pin (this, & basic_cpu::pc_set_pin_handler),
        endian_set_pin (this, & basic_cpu::endian_set_pin_handler),
        debugger_bus (& this->data_bus),
!       trace_stream (),
        trace_filename ("-") // standard output
        {
  	// buses
--- 524,530 ----
        pc_set_pin (this, & basic_cpu::pc_set_pin_handler),
        endian_set_pin (this, & basic_cpu::endian_set_pin_handler),
        debugger_bus (& this->data_bus),
!       trace_stream (std::cout.rdbuf ()),
        trace_filename ("-") // standard output
        {
  	// buses
***************
*** 618,633 ****
      return i;
    }
  
-     template <typename T>
-     basic_cpu::cpu_trace_stream& operator<< (basic_cpu::cpu_trace_stream& s, T t)
-     {
-       if (LIKELY (s.cout_p))
- 	std::cout << t;
-       else
- 	static_cast <std::ofstream&> (s) << t;
-       return s;
-     }
-   
      template <typename BigOrLittleInt>
      BigOrLittleInt basic_cpu::read_insn_memory (sid::host_int_4 pc, sid::host_int_4 address, BigOrLittleInt) const
        {
--- 597,602 ----

Re: PATCH: replace cpu_trace_stream subclass with rdbuf swap

[Frank Ch. Eigler]

Hi -


blandy wrote:

> Below is a patch against the current SID sources to make
> sidutil::basic_cpu::trace_stream into an ordinary std::ostream object,
> which fixes the bug described in my earlier message, but does so by
> removing hair from sidcputil.h (and a bit elsewhere), instead of
> adding to it.

Looks like an improvement, thanks!

> It builds, but I haven't tested it.  The original patch was developed
> on an internal Red Hat branch and tested against an internal Red Hat
> cpu.

Does this version of the code run on that internal port?

> The full SID takes an awful long time to build, and I'm not sure what
> toolchain and BSP to match it against.  Do folks here have any advice
> on how to pare down the SID build and which architecture to target to
> test this with as little fuss as possible?

The usual tricks are to configure sid with a single simple --target (say
m32r-elf), with --disable-shared to omit shared library building, and
build with CXXFLAGS excluding -O2 for a quicker compilation run.


- FChE

Re: PATCH: replace cpu_trace_stream subclass with rdbuf swap

[Jim Blandy]

"Frank Ch. Eigler" <fche@redhat.com> writes:
> > It builds, but I haven't tested it.  The original patch was developed
> > on an internal Red Hat branch and tested against an internal Red Hat
> > cpu.
> 
> Does this version of the code run on that internal port?

Yes --- it works with no redirection, with redirection to a file, with
redirection to one file and then another file (the first file gets
just the "start of trace" line, but is then empty), and redirection to
a file and then back to cout.

> > The full SID takes an awful long time to build, and I'm not sure what
> > toolchain and BSP to match it against.  Do folks here have any advice
> > on how to pare down the SID build and which architecture to target to
> > test this with as little fuss as possible?
> 
> The usual tricks are to configure sid with a single simple --target (say
> m32r-elf), with --disable-shared to omit shared library building, and
> build with CXXFLAGS excluding -O2 for a quicker compilation run.

Thanks --- I'll give that a try.

Re: PATCH: replace cpu_trace_stream subclass with rdbuf swap

[Frank Ch. Eigler]

[-- Attachment #1: Type: text/plain, Size: 258 bytes --]

Hi -


> > Does this version of the code run on that internal port?
> 
> Yes --- it works [with various permutations]

Sounds good.  Please feel free to commit it as soon as you feel
confident that it doesn't break the external version.


- FChE

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: PATCH: replace cpu_trace_stream subclass with rdbuf swap

[Ben Elliston]

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

> > Yes --- it works [with various permutations]
> 
> Sounds good.  Please feel free to commit it as soon as you feel
> confident that it doesn't break the external version.

How about a test case?

Ben