Discussion:
[gem5-dev] Change in gem5/gem5[master]: cpu: Change raw pointers to STL Containers
(too old to reply)
Giacomo Gabrielli (Gerrit)
2018-10-01 08:44:13 UTC
Permalink
Giacomo Gabrielli has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/13126


Change subject: cpu: Change raw pointers to STL Containers
......................................................................

cpu: Change raw pointers to STL Containers

This patch changes two members from being raw pointers to being STL
containers. The reason behind, other than cleanlyness and arguable OO
best practices is that containers have more intronspections capabilities
than naked pointers do, as the size is known.

Using STL containers adds little overhead and eases the automation of
process during debugging (gdb).

Change-Id: I4d9d3eedafa8b5e50ac512ea93b458a4200229f2
Signed-off-by: Giacomo Gabrielli <***@arm.com>
---
M src/cpu/o3/dep_graph.hh
M src/cpu/o3/lsq.hh
M src/cpu/o3/lsq_impl.hh
M src/cpu/o3/lsq_unit.hh
M src/cpu/o3/lsq_unit_impl.hh
5 files changed, 71 insertions(+), 62 deletions(-)



diff --git a/src/cpu/o3/dep_graph.hh b/src/cpu/o3/dep_graph.hh
index 212130e..2e866a6 100644
--- a/src/cpu/o3/dep_graph.hh
+++ b/src/cpu/o3/dep_graph.hh
@@ -122,7 +122,7 @@
* instructions in flight that are dependent upon r34 will be
* in the linked list of dependGraph[34].
*/
- DepEntry *dependGraph;
+ std::vector<DepEntry> dependGraph;

/** Number of linked lists; identical to the number of registers. */
int numEntries;
@@ -140,7 +140,6 @@
template <class DynInstPtr>
DependencyGraph<DynInstPtr>::~DependencyGraph()
{
- delete [] dependGraph;
}

template <class DynInstPtr>
@@ -148,7 +147,7 @@
DependencyGraph<DynInstPtr>::resize(int num_entries)
{
numEntries = num_entries;
- dependGraph = new DepEntry[numEntries];
+ dependGraph.resize(numEntries);
}

template <class DynInstPtr>
diff --git a/src/cpu/o3/lsq.hh b/src/cpu/o3/lsq.hh
index 175821e..cf2b2a4 100644
--- a/src/cpu/o3/lsq.hh
+++ b/src/cpu/o3/lsq.hh
@@ -71,9 +71,7 @@

/** Constructs an LSQ with the given parameters. */
LSQ(O3CPU *cpu_ptr, IEW *iew_ptr, DerivO3CPUParams *params);
- ~LSQ() {
- if (thread) delete [] thread;
- }
+ ~LSQ() { }

/** Returns the name of the LSQ. */
std::string name() const;
@@ -310,8 +308,45 @@
/** The LSQ policy for SMT mode. */
LSQPolicy lsqPolicy;

- /** The LSQ units for individual threads. */
- LSQUnit *thread;
+ /** Transform a SMT sharing policy string into a LSQPolicy value. */
+ static LSQPolicy readLSQPolicy(const std::string& policy) {
+ std::string policy_ = policy;
+ std::transform(policy_.begin(), policy_.end(), policy_.begin(),
+ (int(*)(int)) tolower);
+ if (policy_ == "dynamic") {
+ return Dynamic;
+ } else if (policy_ == "partitioned") {
+ return Partitioned;
+ } else if (policy_ == "threshold") {
+ return Threshold;
+ }
+ assert(0 && "Invalid LSQ Sharing Policy.Options Are:{Dynamic,"
+ "Partitioned, Threshold}");
+
+ // Some compilers complain if there is no return.
+ return Dynamic;
+ }
+
+ /** Auxiliary function to calculate per-thread max LSQ allocation
limit.
+ * Depending on a policy, number of entries and possibly number of
threads
+ * and threshold, this function calculates how many resources each
thread
+ * can occupy at most.
+ */
+ static uint32_t maxLSQAllocation(const LSQPolicy& pol, uint32_t
entries,
+ uint32_t numThreads, uint32_t SMTThreshold) {
+ if (pol == Dynamic) {
+ return entries;
+ } else if (pol == Partitioned) {
+ //@todo:make work if part_amt doesnt divide evenly.
+ return entries / numThreads;
+ } else if (pol == Threshold) {
+ //Divide up by threshold amount
+ //@todo: Should threads check the max and the total
+ //amount of the LSQ
+ return SMTThreshold;
+ }
+ return 0;
+ }

/** List of Active Threads in System. */
std::list<ThreadID> *activeThreads;
@@ -327,6 +362,10 @@
/** Max SQ Size - Used to Enforce Sharing Policies. */
unsigned maxSQEntries;

+ using LSQUnitAllocator = typename LSQUnit::Allocator;
+ /** The LSQ units for individual threads. */
+ std::vector<LSQUnit,LSQUnitAllocator> thread;
+
/** Number of Threads. */
ThreadID numThreads;
};
diff --git a/src/cpu/o3/lsq_impl.hh b/src/cpu/o3/lsq_impl.hh
index 967a496..49330b2 100644
--- a/src/cpu/o3/lsq_impl.hh
+++ b/src/cpu/o3/lsq_impl.hh
@@ -60,8 +60,14 @@
template <class Impl>
LSQ<Impl>::LSQ(O3CPU *cpu_ptr, IEW *iew_ptr, DerivO3CPUParams *params)
: cpu(cpu_ptr), iewStage(iew_ptr),
+ lsqPolicy(readLSQPolicy(params->smtLSQPolicy)),
LQEntries(params->LQEntries),
SQEntries(params->SQEntries),
+ maxLQEntries(maxLSQAllocation(lsqPolicy, LQEntries,
params->numThreads,
+ params->smtLSQThreshold)),
+ maxSQEntries(maxLSQAllocation(lsqPolicy, SQEntries,
params->numThreads,
+ params->smtLSQThreshold)),
+ thread(LSQUnitAllocator(maxLQEntries, maxSQEntries)),
numThreads(params->numThreads)
{
assert(numThreads > 0 && numThreads <= Impl::MaxThreads);
@@ -69,55 +75,27 @@
//**********************************************/
//************ Handle SMT Parameters ***********/
//**********************************************/
- std::string policy = params->smtLSQPolicy;
-
- //Convert string to lowercase
- std::transform(policy.begin(), policy.end(), policy.begin(),
- (int(*)(int)) tolower);

//Figure out fetch policy
- if (policy == "dynamic") {
- lsqPolicy = Dynamic;
-
- maxLQEntries = LQEntries;
- maxSQEntries = SQEntries;
-
+ if (lsqPolicy == Dynamic) {
DPRINTF(LSQ, "LSQ sharing policy set to Dynamic\n");
- } else if (policy == "partitioned") {
- lsqPolicy = Partitioned;
-
- //@todo:make work if part_amt doesnt divide evenly.
- maxLQEntries = LQEntries / numThreads;
- maxSQEntries = SQEntries / numThreads;
-
+ } else if (lsqPolicy == Partitioned) {
DPRINTF(Fetch, "LSQ sharing policy set to Partitioned: "
"%i entries per LQ | %i entries per SQ\n",
maxLQEntries,maxSQEntries);
- } else if (policy == "threshold") {
- lsqPolicy = Threshold;
+ } else if (lsqPolicy == Threshold) {

assert(params->smtLSQThreshold > LQEntries);
assert(params->smtLSQThreshold > SQEntries);

- //Divide up by threshold amount
- //@todo: Should threads check the max and the total
- //amount of the LSQ
- maxLQEntries = params->smtLSQThreshold;
- maxSQEntries = params->smtLSQThreshold;
-
DPRINTF(LSQ, "LSQ sharing policy set to Threshold: "
"%i entries per LQ | %i entries per SQ\n",
maxLQEntries,maxSQEntries);
- } else {
- assert(0 && "Invalid LSQ Sharing Policy.Options Are:{Dynamic,"
- "Partitioned, Threshold}");
}
+ this->thread.resize(params->numThreads);

- //Initialize LSQs
- thread = new LSQUnit[numThreads];
for (ThreadID tid = 0; tid < numThreads; tid++) {
- thread[tid].init(cpu, iew_ptr, params, this,
- maxLQEntries, maxSQEntries, tid);
+ thread[tid].init(cpu, iew_ptr, params, this, tid);
thread[tid].setDcachePort(&cpu_ptr->getDataPort());
}
}
diff --git a/src/cpu/o3/lsq_unit.hh b/src/cpu/o3/lsq_unit.hh
index 4084f3b..5b5cae3 100644
--- a/src/cpu/o3/lsq_unit.hh
+++ b/src/cpu/o3/lsq_unit.hh
@@ -54,6 +54,7 @@
#include "arch/isa_traits.hh"
#include "arch/locked_mem.hh"
#include "arch/mmapped_ipr.hh"
+#include "base/allocators.hh"
#include "config/the_isa.hh"
#include "cpu/inst_seq.hh"
#include "cpu/timebuf.hh"
@@ -84,15 +85,17 @@
typedef typename Impl::CPUPol::IEW IEW;
typedef typename Impl::CPUPol::LSQ LSQ;
typedef typename Impl::CPUPol::IssueStruct IssueStruct;
+ using Allocator = ArgAllocator<LSQUnit,
+ FixedArg<uint32_t>,
+ FixedArg<uint32_t>>;

public:
/** Constructs an LSQ unit. init() must be called prior to use. */
- LSQUnit();
+ LSQUnit(uint32_t lqEntries, uint32_t sqEntries);

/** Initializes the LSQ unit with the specified number of entries. */
void init(O3CPU *cpu_ptr, IEW *iew_ptr, DerivO3CPUParams *params,
- LSQ *lsq_ptr, unsigned maxLQEntries, unsigned maxSQEntries,
- unsigned id);
+ LSQ *lsq_ptr, unsigned id);

/** Returns the name of the LSQ unit. */
std::string name() const;
diff --git a/src/cpu/o3/lsq_unit_impl.hh b/src/cpu/o3/lsq_unit_impl.hh
index d8f1c39..5438c4d 100644
--- a/src/cpu/o3/lsq_unit_impl.hh
+++ b/src/cpu/o3/lsq_unit_impl.hh
@@ -142,8 +142,10 @@
}

template <class Impl>
-LSQUnit<Impl>::LSQUnit()
- : loads(0), stores(0), storesToWB(0), cacheBlockMask(0),
stalled(false),
+LSQUnit<Impl>::LSQUnit(uint32_t lqEntries, uint32_t sqEntries)
+ : lsqID(-1), storeQueue(sqEntries+1), loadQueue(lqEntries+1),
+ LQEntries(lqEntries+1), SQEntries(lqEntries+1),
+ loads(0), stores(0), storesToWB(0), cacheBlockMask(0),
stalled(false),
isStoreBlocked(false), storeInFlight(false), hasPendingPkt(false),
pendingPkt(nullptr)
{
@@ -152,28 +154,16 @@
template<class Impl>
void
LSQUnit<Impl>::init(O3CPU *cpu_ptr, IEW *iew_ptr, DerivO3CPUParams *params,
- LSQ *lsq_ptr, unsigned maxLQEntries, unsigned maxSQEntries,
- unsigned id)
+ LSQ *lsq_ptr, unsigned id)
{
+ lsqID = id;
+
cpu = cpu_ptr;
iewStage = iew_ptr;

lsq = lsq_ptr;

- lsqID = id;
-
- DPRINTF(LSQUnit, "Creating LSQUnit%i object.\n",id);
-
- // Add 1 for the sentinel entry (they are circular queues).
- LQEntries = maxLQEntries + 1;
- SQEntries = maxSQEntries + 1;
-
- //Due to uint8_t index in LSQSenderState
- assert(LQEntries <= 256);
- assert(SQEntries <= 256);
-
- loadQueue.resize(LQEntries);
- storeQueue.resize(SQEntries);
+ DPRINTF(LSQUnit, "Creating LSQUnit%i object.\n",lsqID);

depCheckShift = params->LSQDepCheckShift;
checkLoads = params->LSQCheckLoads;
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/13126
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-Change-Id: I4d9d3eedafa8b5e50ac512ea93b458a4200229f2
Gerrit-Change-Number: 13126
Gerrit-PatchSet: 1
Gerrit-Owner: Giacomo Gabrielli <***@arm.com>
Gerrit-MessageType: newchange
Giacomo Gabrielli (Gerrit)
2018-10-05 15:32:07 UTC
Permalink
Giacomo Gabrielli has uploaded a new patch set (#5). (
https://gem5-review.googlesource.com/c/public/gem5/+/13126 )

Change subject: cpu: Change raw pointers to STL Containers
......................................................................

cpu: Change raw pointers to STL Containers

This patch changes two members from being raw pointers to being STL
containers. The reason behind, other than cleanlyness and arguable OO
best practices is that containers have more intronspections capabilities
than naked pointers do, as the size is known.

Using STL containers adds little overhead and eases the automation of
process during debugging (gdb).

Change-Id: I4d9d3eedafa8b5e50ac512ea93b458a4200229f2
Signed-off-by: Giacomo Gabrielli <***@arm.com>
---
M src/cpu/o3/dep_graph.hh
M src/cpu/o3/lsq.hh
M src/cpu/o3/lsq_impl.hh
M src/cpu/o3/lsq_unit.hh
M src/cpu/o3/lsq_unit_impl.hh
5 files changed, 72 insertions(+), 62 deletions(-)
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/13126
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-Change-Id: I4d9d3eedafa8b5e50ac512ea93b458a4200229f2
Gerrit-Change-Number: 13126
Gerrit-PatchSet: 5
Gerrit-Owner: Giacomo Gabrielli <***@arm.com>
Gerrit-MessageType: newpatchset
Giacomo Travaglini (Gerrit)
2018-11-28 15:15:00 UTC
Permalink
Giacomo Travaglini has uploaded a new patch set (#12) to the change
originally created by Giacomo Gabrielli. (
https://gem5-review.googlesource.com/c/public/gem5/+/13126 )

Change subject: cpu: Change raw pointers to STL Containers
......................................................................

cpu: Change raw pointers to STL Containers

This patch changes two members from being raw pointers to being STL
containers. The reason behind, other than cleanlyness and arguable OO
best practices is that containers have more intronspections capabilities
than naked pointers do, as the size is known.

Using STL containers adds little overhead and eases the automation of
process during debugging (gdb).

Change-Id: I4d9d3eedafa8b5e50ac512ea93b458a4200229f2
Signed-off-by: Giacomo Gabrielli <***@arm.com>
---
M src/cpu/o3/dep_graph.hh
M src/cpu/o3/lsq.hh
M src/cpu/o3/lsq_impl.hh
M src/cpu/o3/lsq_unit.hh
M src/cpu/o3/lsq_unit_impl.hh
5 files changed, 72 insertions(+), 59 deletions(-)
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/13126
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-Change-Id: I4d9d3eedafa8b5e50ac512ea93b458a4200229f2
Gerrit-Change-Number: 13126
Gerrit-PatchSet: 12
Gerrit-Owner: Giacomo Gabrielli <***@arm.com>
Gerrit-Reviewer: Andreas Sandberg <***@arm.com>
Gerrit-Reviewer: Giacomo Travaglini <***@arm.com>
Gerrit-Reviewer: Jason Lowe-Power <***@lowepower.com>
Gerrit-CC: Brandon Potter <***@amd.com>
Gerrit-CC: Gabor Dozsa <***@arm.com>
Gerrit-MessageType: newpatchset
Continue reading on narkive:
Loading...