cpu: Dynamically instantiate O3 CPU LSQUnits

Information
Submitter:	Joel Hestness
Repository:	gem5
Branch:	default
Bugs:
Depends On:
Reviewers
Groups:	Default
People:

Description

Changeset 9818:132e2bbe723e
---------------------------
cpu: Dynamically instantiate O3 CPU LSQUnits

Previously, the LSQ would instantiate MaxThreads LSQUnits in the body of it's
object, but it would only initialize numThreads LSQUnits as specified by the
user. This had the effect of leaving some LSQUnits uninitialized when the
number of threads was less than MaxThreads, and when adding statistics to the
LSQUnit that must be initialized, this caused the stats initialization check to
fail. By dynamically instantiating LSQUnits, they are all initialized and this
avoids uninitialized LSQUnits from floating around during runtime.

Testing Done

Ran O3 regressions and tested checkpoint restore with additional LSQUnit stats.

Issue Summary

Description	From	Last Updated	Status
Could this perhaps be a std::vector<LSQUnit*> rather?	Andreas Hansson	Sept. 9, 2013, 12:47 p.m.	Dropped
If it's a vector, then you can do this in the initalisation list and the delete goes away.	Andreas Hansson	Sept. 9, 2013, 12:47 p.m.	Dropped
My bad, std::vector<LSQUnit> is what I was thinking	Andreas Hansson	Sept. 9, 2013, 12:47 p.m.	Dropped

src/cpu/o3/lsq.hh (Diff revision 1)

Could this perhaps be a std::vector<LSQUnit*> rather?

Show all issues

Joel Hestness July 19, 2013, 1:58 p.m. (July 19, 2013, 1:58 p.m.)

Sure, though we'd still need to dynamically allocate the LSQUnits in the LSQ constructor loop, and change all of the "thread[tid]." to dereference the appropriate LSQUnits unless we use pointer references. This would also fail to eliminate the LSQ destructor unless we explicitly templatize the vector to call the LSQUnit destructor. If we dynamically allocate, we'll need to call a destructor somewhere. The vector route seems pretty messy unless I'm missing something you're saying here?

src/cpu/o3/lsq_impl.hh (Diff revision 1)

If it's a vector, then you can do this in the initalisation list and the delete goes away.

Show all issues

Joel Hestness July 19, 2013, 1:58 p.m. (July 19, 2013, 1:58 p.m.)

src/cpu/o3/lsq.hh (Diff revision 1)

My bad, std::vector<LSQUnit> is what I was thinking

Show all issues

Joel Hestness July 20, 2013, 2:49 p.m. (July 20, 2013, 2:49 p.m.)

Ah, I see.  I just tried this, and there are issues with the stats allocators that cause failed linking.  In particular, when instantiating multiple LSQUnits into this vector in the initialization list of the LSQ constructor, g++ instantiates a single LSQUnit and then calls the LSQUnit copy constructor to create the rest in the vector.  These calls to the LSQUnit copy constructor call the DataWrap copy constructor for each of the stats.  This fails because the stats DataWrap copy constructor is a stub intended to fail linking if it is called (see src/base/statistics.hh:231) - presumably to keep devs from dynamically allocating stats within a SimObject or within an STL container that might copy them.

I've played around with the DataWrap copy constructor just to get things to build and run, and we still run into the bug that this patch is intended to fix, namely, there are LSQUnits that are allocated but their stats are not initialized.  It appears this is because the standard STL vector allocator is allowed to allocate more objects than are necessarily used to allow for easily increasing the size of the vector without reallocating space.  I think we'd need to write a custom allocator in order to use std::vector<LSQUnit>.

Andreas Hansson July 21, 2013, 6:31 a.m. (July 21, 2013, 6:31 a.m.)

Wow, thanks for all the digging. I forgot about the vector behaviour.

Keep it simple then :-)

LGTM

You have a pending review.

Review Board 2.0.15

This change has been marked as submitted.

Screenshots

Files

Issue Summary

Status: Closed (submitted)