Hello Andreas Sandberg,

I'd like you to do a code review. Please visit

    https://gem5-review.googlesource.com/8062

to review the following change.


Change subject: sim: Remove _numContexts member in System class
......................................................................

sim: Remove _numContexts member in System class

A System object has a _numContexts member variable which represent the
number of ThreadContext registered in the System.  Since this has to
match the size of the ThreadContext vector, this patch removes the
manually cached size. This was usually used as a for-loop index, whereas
we want to enforce the use of range-based loops whenever possible.

Change-Id: I1ba317c0393bcc9c1aeebbb1fc22d7b2bc2cf90c
Signed-off-by: Giacomo Travaglini <giacomo.travagl...@arm.com>
Reviewed-by: Andreas Sandberg <andreas.sandb...@arm.com>
---
M src/arch/arm/kvm/gic.cc
M src/arch/arm/linux/system.cc
M src/sim/system.cc
M src/sim/system.hh
4 files changed, 13 insertions(+), 20 deletions(-)



diff --git a/src/arch/arm/kvm/gic.cc b/src/arch/arm/kvm/gic.cc
index 887fa8d..d2423a6 100644
--- a/src/arch/arm/kvm/gic.cc
+++ b/src/arch/arm/kvm/gic.cc
@@ -305,7 +305,7 @@
MuxingKvmGic::copyBankedDistRange(BaseGicRegisters* from, BaseGicRegisters* to,
                                   Addr daddr, size_t size)
 {
-    for (int ctx = 0; ctx < system._numContexts; ++ctx)
+    for (int ctx = 0; ctx < system.numContexts(); ++ctx)
         for (auto a = daddr; a < daddr + size; a += 4)
             copyDistRegister(from, to, ctx, a);
 }
@@ -314,7 +314,7 @@
 MuxingKvmGic::clearBankedDistRange(BaseGicRegisters* to,
                                    Addr daddr, size_t size)
 {
-    for (int ctx = 0; ctx < system._numContexts; ++ctx)
+    for (int ctx = 0; ctx < system.numContexts(); ++ctx)
         for (auto a = daddr; a < daddr + size; a += 4)
             to->writeDistributor(ctx, a, 0xFFFFFFFF);
 }
@@ -345,7 +345,7 @@
     // Copy CPU Interface Control Register (CTLR),
     //      Interrupt Priority Mask Register (PMR), and
     //      Binary Point Register (BPR)
-    for (int ctx = 0; ctx < system._numContexts; ++ctx) {
+    for (int ctx = 0; ctx < system.numContexts(); ++ctx) {
         copyCpuRegister(from, to, ctx, GICC_CTLR);
         copyCpuRegister(from, to, ctx, GICC_PMR);
         copyCpuRegister(from, to, ctx, GICC_BPR);
@@ -423,7 +423,7 @@
     // have been shifted by three bits due to its having been emulated by
     // a VGIC with only 5 PMR bits in its VMCR register.  Presently the
     // Linux kernel does not repair this inaccuracy, so we correct it here.
-    for (int cpu = 0; cpu < system._numContexts; ++cpu) {
+    for (int cpu = 0; cpu < system.numContexts(); ++cpu) {
        cpuPriority[cpu] <<= 3;
        assert((cpuPriority[cpu] & ~0xff) == 0);
     }
diff --git a/src/arch/arm/linux/system.cc b/src/arch/arm/linux/system.cc
index 7f06475..094e4d7 100644
--- a/src/arch/arm/linux/system.cc
+++ b/src/arch/arm/linux/system.cc
@@ -250,8 +250,7 @@
         std::string task_filename = "tasks.txt";
         taskFile = simout.create(name() + "." + task_filename);

-        for (int i = 0; i < _numContexts; i++) {
-            ThreadContext *tc = threadContexts[i];
+        for (const auto tc : threadContexts) {
             uint32_t pid = tc->getCpuPtr()->getPid();
             if (pid != BaseCPU::invldPid) {
                 mapPid(tc, pid);
diff --git a/src/sim/system.cc b/src/sim/system.cc
index ed01e0e..4541d77 100644
--- a/src/sim/system.cc
+++ b/src/sim/system.cc
@@ -87,7 +87,6 @@

 System::System(Params *p)
     : MemObject(p), _systemPort("system_port", this),
-      _numContexts(0),
       multiThread(p->multi_thread),
       pagePtr(0),
       init_param(p->init_param),
@@ -256,7 +255,6 @@
              "Cannot have two CPUs with the same id (%d)\n", id);

     threadContexts[id] = tc;
-    _numContexts++;

 #if THE_ISA != NULL_ISA
     int port = getRemoteGDBPort();
@@ -287,12 +285,13 @@
 int
 System::numRunningContexts()
 {
-    int running = 0;
-    for (int i = 0; i < _numContexts; ++i) {
-        if (threadContexts[i]->status() != ThreadContext::Halted)
-            ++running;
-    }
-    return running;
+    return std::count_if(
+        threadContexts.cbegin(),
+        threadContexts.cend(),
+        [] (ThreadContext* tc) {
+            return tc->status() != ThreadContext::Halted;
+        }
+    );
 }

 void
diff --git a/src/sim/system.hh b/src/sim/system.hh
index 5b0c178..a72f2a7 100644
--- a/src/sim/system.hh
+++ b/src/sim/system.hh
@@ -196,7 +196,6 @@
 #endif

     std::vector<ThreadContext *> threadContexts;
-    int _numContexts;
     const bool multiThread;

     ThreadContext *getThreadContext(ContextID tid)
@@ -204,11 +203,7 @@
         return threadContexts[tid];
     }

-    int numContexts()
-    {
-        assert(_numContexts == (int)threadContexts.size());
-        return _numContexts;
-    }
+    unsigned numContexts() const { return threadContexts.size(); }

     /** Return number of running (non-halted) thread contexts in
      * system.  These threads could be Active or Suspended. */

--
To view, visit https://gem5-review.googlesource.com/8062
To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1ba317c0393bcc9c1aeebbb1fc22d7b2bc2cf90c
Gerrit-Change-Number: 8062
Gerrit-PatchSet: 1
Gerrit-Owner: Giacomo Travaglini <giacomo.travagl...@arm.com>
Gerrit-Reviewer: Andreas Sandberg <andreas.sandb...@arm.com>
_______________________________________________
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to