misc: Separate stats file is generated for SystemC-gem5 co-simulation

Information
Submitter:	Abdul Mutaal Ahmad
Repository:	gem5
Branch:
Bugs:
Depends On:
Reviewers
Groups:	Default
People:

Description

In previous versions of systemC-gem5 coupling statistics were not updated for the systemc-gem5 simulation. systemC-gem5 simulation only need the previously built config.ini file and normal gem5 simulation has to be run once to generate config.ini file. Thus stats.txt inside the m5out folder is redundant for systemC-gem5 simulation. A new stats file is now generated with the all the statistics for systemC-gem5 simulation. This will also resolve the stats issue in tlm-sysmtemC simulation.

Testing Done

Issue Summary

Description	From	Last Updated	Status
I think a more chatty error message would be beneficial.	Matthias Jung	Sept. 24, 2015, 2:38 a.m.	Resolved
The file name can be kept as constant, but at least pass an argument to this function that specifies the ...	Nilay Vaish	June 30, 2016, 12:41 a.m.	Resolved

util/systemc/stats.cc (Diff revision 1)

I think a more chatty error message would be beneficial.

Show all issues

Change Summary:

Added warning if particular stat is not dumped

Diff:

Revision 2 (+65 -20)

Show changes

	util/systemc/main.cc
	util/systemc/stats.hh
	util/systemc/stats.cc
	util/tlm/main.cc

Summary:

-	Separate stats file is generated for SystemC-gem5 co-simulation
+	mem:Separate stats file is generated for SystemC-gem5 co-simulation

Summary:

-	mem:Separate stats file is generated for SystemC-gem5 co-simulation
+	misc: Separate stats file is generated for SystemC-gem5 co-simulation

Ship It!

Format this patch again. Read the following page: http://gem5.org/Submitting_Contributions

Nilay Vaish Oct. 14, 2015, 6:03 a.m. (Oct. 14, 2015, 6:03 a.m.)

Several lines are more than 80 characters. Also make sure that
the identation is correct.

Change Summary:

improved identation and 80 character limit

Diff:

Revision 3 (+72 -20)

Show changes

	util/systemc/main.cc
	util/systemc/stats.hh
	util/systemc/stats.cc
	util/tlm/main.cc

Overall the patch seems fine to me. Just take care of the concern mentioned below.

util/systemc/stats.cc (Diff revision 3)

The file name can be kept as constant, but at least pass an argument to this function that specifies the file path.

I would prefer if you use the same output directory that the rest of the system is using.

Show all issues

Abdul Mutaal Ahmad Nov. 26, 2015, 7:45 a.m. (Nov. 26, 2015, 7:45 a.m.)

are you suggesting to use OutputDirectory class and get the current directory create a stats file there. As far as I know the current management of stats is wrapped inside the python calls

Nilay Vaish Dec. 1, 2015, 10:30 a.m. (Dec. 1, 2015, 10:30 a.m.)

Forget about using the OutputDirectory class for the time being.  Is it at all possible for statsDump

to receive a file path in which statistics are going to be dumped?  I would not assume that m5out 

directory is present in cwd.

Abdul Mutaal Ahmad Dec. 2, 2015, 4:40 a.m. (Dec. 2, 2015, 4:40 a.m.)

It is possible but that will eventually need some changes in statistics.cc/statistics.hh . statsDump and statsReset are handlers and they are registered with statistics using registerHandlers defined in statistics.hh. I don't think it's a good solution beacuse it will disturb the python calls for normal gem5 run.

Andrew Bardsley May 19, 2016, 7:09 a.m. (May 19, 2016, 7:09 a.m.)

As the whole contents of util/systemc is intended to be just an example of how to construct a top level to use the .ini loading mechanism within a 'foreign' SystemC simulation, I'd suggest passing a filename down from main.cc as it's not unlikely that the user of this example would have their own, application specific, ideas about where the stats file should end up that doesn't sensibly involve reusing any of the Python mechanisms.
Perhaps add a global string variable to stats.{hh,cc} in the first instance and set it from main.cc (to make the point about that the files other than main.cc should be progressing towards being a SystemC interfacing library with main.cc as a user of that library).
Looking at the registerHandlers mechanism. I think perhaps that should be changed to use a 'DumpOperations' class. An object of a derived class of DumpOperations can then be registered with registerHandlers.  Call made on that object to reset/dump stats can then be made rather than on raw function pointers (mea culpa (I think)). With an object handling stats dumping, local data could then be held (including a filename). Maybe that should be a second step in another patch?

It is not clear to me why this is not just calling the "normal" stats dump. It seems like an unfortunate amount of code duplication.

Abdul Mutaal Ahmad Nov. 21, 2015, 8:05 a.m. (Nov. 21, 2015, 8:05 a.m.)

It is using the same API as used in util/cxx_config. Even the util/systemc is built on the same code. Since systemC runs need 2 parts, first to generate the config.ini and then run it within systemC. The only thing which seems logical is to use same stats.txt which was used in the first part to generate config.ini. The "normal" stats are calculated through python calls.

Andrew Bardsley May 31, 2016, 2:19 a.m. (May 31, 2016, 2:19 a.m.)

The two examples should be developed to share a C++ mechanism for dumping stats which should probably then be pushed back into the normal codebase (out of util/...). I haven't been following developments in stats and so can't really comment on how much Python is still required by them. Both util/cxx_config/stats.{cc,hh} and util/systemc/stats.{cc,hh} were offered as proof-of-principle examples.

I think the presence of the C++ stats dumping code in this patch takes the discussion one step on by offering a usable implementation. It may be that C++ and Python-involving stats can't converge but this patch at least provides a route for Python-less gem5 users to make use of stats in the same form. I think the code offered here still represents a significant improvement on the original util/{cxx_config,systemc}/stats.{cpp,h}. (Full disclosure: I've not read the cascading dynamic_cast in any detail nor tried running it)

Caveat: make the filename/configuration for stats output visible in the top-level main.cc.

Change Summary:

A global 'filename'variable is added in stats.hh that is set from main.cc.

Diff:

Revision 4 (+84 -20)

Show changes

	util/systemc/main.cc
	util/systemc/stats.hh
	util/systemc/stats.cc
	util/tlm/main.cc

Ship It!

You have a pending review.

Review Board 2.0.15

This change has been marked as submitted.

Screenshots

Files

Issue Summary

Change Summary:

Diff:

Summary:

Summary:

Change Summary:

Diff:

Change Summary:

Diff:

Status: Closed (submitted)