Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/32214 )

Change subject: cpu: Remove the "profile" parameter and plumbing.
......................................................................

cpu: Remove the "profile" parameter and plumbing.

This parameter is associated with a periodic event which would take a
sample for a kernel profile in FS mode. Unfortunately the only ISA which
had working versions of the necessary classes was alpha, and that has
been deleted. That means that without additional work for any given ISA,
the profile parameter has no chance of working.

Ideally, this parameter should be moved to the Workload classes. There
it can intrinsically be tied to a particular kernel, rather than having
to assume a particular kernel and gate everything on whether you're in
FS mode.

Because this isn't (IMHO) where this parameter should live in the long
term, and because it's currently unusable without additional development
for each of the ISAs, I think it makes the most sense to remove the
front end for this mechanism from the CPU.

Since the sampling/profiling mechanism itself could be useful and could
be re-plumbed somewhere else, the back end and its classes are left alone.

Change-Id: I2a3319c1d5ad0ef8c99f5d35953b93c51b2a8a0b
---
M src/arch/arm/fastmodel/iris/thread_context.hh
M src/cpu/BaseCPU.py
M src/cpu/base.cc
M src/cpu/base.hh
M src/cpu/checker/thread_context.hh
M src/cpu/o3/commit_impl.hh
M src/cpu/o3/thread_context.hh
M src/cpu/o3/thread_context_impl.hh
M src/cpu/o3/thread_state.hh
M src/cpu/simple/base.cc
M src/cpu/simple_thread.cc
M src/cpu/simple_thread.hh
M src/cpu/thread_context.hh
M src/cpu/thread_state.cc
M src/cpu/thread_state.hh
15 files changed, 1 insertion(+), 211 deletions(-)



diff --git a/src/arch/arm/fastmodel/iris/thread_context.hh b/src/arch/arm/fastmodel/iris/thread_context.hh
index 8386e23..363e1d7 100644
--- a/src/arch/arm/fastmodel/iris/thread_context.hh
+++ b/src/arch/arm/fastmodel/iris/thread_context.hh
@@ -238,12 +238,6 @@
     void halt() override { setStatus(Halted); }

     void
-    dumpFuncProfile() override
-    {
-        panic("%s not implemented.", __FUNCTION__);
-    }
-
-    void
     takeOverFrom(::ThreadContext *old_context) override
     {
         panic("%s not implemented.", __FUNCTION__);
@@ -264,17 +258,6 @@
     }

     void
-    profileClear() override
-    {
-        panic("%s not implemented.", __FUNCTION__);
-    }
-    void
-    profileSample() override
-    {
-        panic("%s not implemented.", __FUNCTION__);
-    }
-
-    void
     copyArchRegs(::ThreadContext *tc) override
     {
         panic("%s not implemented.", __FUNCTION__);
diff --git a/src/cpu/BaseCPU.py b/src/cpu/BaseCPU.py
index e487cbb..96e96fc 100644
--- a/src/cpu/BaseCPU.py
+++ b/src/cpu/BaseCPU.py
@@ -148,8 +148,6 @@
     do_statistics_insts = Param.Bool(True,
         "enable statistics pseudo instructions")

-    profile = Param.Latency('0ns', "trace the kernel stack")
-
     wait_for_remote_gdb = Param.Bool(False,
         "Wait for a remote GDB connection");

diff --git a/src/cpu/base.cc b/src/cpu/base.cc
index c903145..084c1e5 100644
--- a/src/cpu/base.cc
+++ b/src/cpu/base.cc
@@ -54,7 +54,6 @@
 #include "base/output.hh"
 #include "base/trace.hh"
 #include "cpu/checker/cpu.hh"
-#include "cpu/profile.hh"
 #include "cpu/thread_context.hh"
 #include "debug/Mwait.hh"
 #include "debug/SyscallVerbose.hh"
@@ -127,8 +126,7 @@
       _dataMasterId(p->system->getMasterId(this, "data")),
       _taskId(ContextSwitchTaskId::Unknown), _pid(invldPid),
_switchedOut(p->switched_out), _cacheLineSize(p->system->cacheLineSize()),
-      interrupts(p->interrupts), profileEvent(NULL),
-      numThreads(p->numThreads), system(p->system),
+ interrupts(p->interrupts), numThreads(p->numThreads), system(p->system),
       previousCycle(0), previousState(CPU_STATE_SLEEP),
       functionTraceStream(nullptr), currentFunctionStart(0),
       currentFunctionEnd(0), functionEntryTick(0),
@@ -169,12 +167,6 @@
         }
     }

-    if (FullSystem) {
-        if (params()->profile)
-            profileEvent = new EventFunctionWrapper(
-                [this]{ processProfileEvent(); },
-                name());
-    }
     tracer = params()->tracer;

     if (params()->isa.size() != numThreads) {
@@ -191,7 +183,6 @@

 BaseCPU::~BaseCPU()
 {
-    delete profileEvent;
 }

 void
@@ -307,11 +298,6 @@
 void
 BaseCPU::startup()
 {
-    if (FullSystem) {
-        if (!params()->switched_out && profileEvent)
-            schedule(profileEvent, curTick());
-    }
-
     if (params()->progress_interval) {
         new CPUProgressEvent(this, params()->progress_interval);
     }
@@ -536,8 +522,6 @@
 {
     assert(!_switchedOut);
     _switchedOut = true;
-    if (profileEvent && profileEvent->scheduled())
-        deschedule(profileEvent);

     // Flush all TLBs in the CPU to avoid having stale translations if
     // it gets switched in later.
@@ -628,14 +612,6 @@
     }
     oldCPU->interrupts.clear();

-    if (FullSystem) {
-        for (ThreadID i = 0; i < size; ++i)
-            threadContexts[i]->profileClear();
-
-        if (profileEvent)
-            schedule(profileEvent, curTick());
-    }
-
     // All CPUs have an instruction and a data port, and the new CPU's
     // ports are dangling while the old CPU has its ports connected
     // already. Unbind the old CPU and then bind the ports of the one
@@ -661,17 +637,6 @@
 }

 void
-BaseCPU::processProfileEvent()
-{
-    ThreadID size = threadContexts.size();
-
-    for (ThreadID i = 0; i < size; ++i)
-        threadContexts[i]->profileSample();
-
-    schedule(profileEvent, curTick() + params()->profile);
-}
-
-void
 BaseCPU::serialize(CheckpointOut &cp) const
 {
     SERIALIZE_SCALAR(instCnt);
diff --git a/src/cpu/base.hh b/src/cpu/base.hh
index b9456a9..2b4ac3c 100644
--- a/src/cpu/base.hh
+++ b/src/cpu/base.hh
@@ -257,9 +257,6 @@
         return FullSystem && interrupts[tid]->checkInterrupts();
     }

-    void processProfileEvent();
-    EventFunctionWrapper * profileEvent;
-
   protected:
     std::vector<ThreadContext *> threadContexts;

diff --git a/src/cpu/checker/thread_context.hh b/src/cpu/checker/thread_context.hh
index 6205222..06cb3b9 100644
--- a/src/cpu/checker/thread_context.hh
+++ b/src/cpu/checker/thread_context.hh
@@ -196,8 +196,6 @@
     /// Set the status to Halted.
     void halt() override { actualTC->halt(); }

-    void dumpFuncProfile() override { actualTC->dumpFuncProfile(); }
-
     void
     takeOverFrom(ThreadContext *oldContext) override
     {
@@ -215,9 +213,6 @@
Tick readLastActivate() override { return actualTC->readLastActivate(); }
     Tick readLastSuspend() override { return actualTC->readLastSuspend(); }

-    void profileClear() override { return actualTC->profileClear(); }
-    void profileSample() override { return actualTC->profileSample(); }
-
     // @todo: Do I need this?
     void
     copyArchRegs(ThreadContext *tc) override
diff --git a/src/cpu/o3/commit_impl.hh b/src/cpu/o3/commit_impl.hh
index 49b40e3..c170484 100644
--- a/src/cpu/o3/commit_impl.hh
+++ b/src/cpu/o3/commit_impl.hh
@@ -1281,14 +1281,6 @@
     updateComInstStats(head_inst);

     if (FullSystem) {
-        if (thread[tid]->profile) {
-            thread[tid]->profilePC = head_inst->instAddr();
-            ProfileNode *node = thread[tid]->profile->consume(
-                    thread[tid]->getTC(), head_inst->staticInst);
-
-            if (node)
-                thread[tid]->profileNode = node;
-        }
         if (CPA::available()) {
             if (head_inst->isControl()) {
                 ThreadContext *tc = thread[tid]->getTC();
diff --git a/src/cpu/o3/thread_context.hh b/src/cpu/o3/thread_context.hh
index b1c47d2..f710613 100644
--- a/src/cpu/o3/thread_context.hh
+++ b/src/cpu/o3/thread_context.hh
@@ -172,11 +172,6 @@
     /** Set the status to Halted. */
     void halt() override;

-    /** Dumps the function profiling information.
-     * @todo: Implement.
-     */
-    void dumpFuncProfile() override;
-
     /** Takes over execution of a thread from another CPU. */
     void takeOverFrom(ThreadContext *old_context) override;

@@ -185,11 +180,6 @@
     /** Reads the last tick that this thread was suspended on. */
     Tick readLastSuspend() override;

-    /** Clears the function profiling information. */
-    void profileClear() override;
-    /** Samples the function profiling information. */
-    void profileSample() override;
-
     /** Copies the architectural registers from another TC into this TC. */
     void copyArchRegs(ThreadContext *tc) override;

diff --git a/src/cpu/o3/thread_context_impl.hh b/src/cpu/o3/thread_context_impl.hh
index 588db15..a5db764 100644
--- a/src/cpu/o3/thread_context_impl.hh
+++ b/src/cpu/o3/thread_context_impl.hh
@@ -57,13 +57,6 @@

 template <class Impl>
 void
-O3ThreadContext<Impl>::dumpFuncProfile()
-{
-    thread->dumpFuncProfile();
-}
-
-template <class Impl>
-void
 O3ThreadContext<Impl>::takeOverFrom(ThreadContext *old_context)
 {
     ::takeOverFrom(*this, *old_context);
@@ -155,20 +148,6 @@

 template <class Impl>
 void
-O3ThreadContext<Impl>::profileClear()
-{
-    thread->profileClear();
-}
-
-template <class Impl>
-void
-O3ThreadContext<Impl>::profileSample()
-{
-    thread->profileSample();
-}
-
-template <class Impl>
-void
 O3ThreadContext<Impl>::copyArchRegs(ThreadContext *tc)
 {
     // Set vector renaming mode before copying registers
diff --git a/src/cpu/o3/thread_state.hh b/src/cpu/o3/thread_state.hh
index 30320bc..757671a 100644
--- a/src/cpu/o3/thread_state.hh
+++ b/src/cpu/o3/thread_state.hh
@@ -41,7 +41,6 @@
 #ifndef __CPU_O3_THREAD_STATE_HH__
 #define __CPU_O3_THREAD_STATE_HH__

-#include "arch/stacktrace.hh"
 #include "base/callback.hh"
 #include "base/compiler.hh"
 #include "base/output.hh"
@@ -52,9 +51,7 @@

 class Event;
 class FunctionalMemory;
-class FunctionProfile;
 class Process;
-class ProfileNode;

 /**
  * Class that has various thread state, such as the status, the
@@ -100,24 +97,6 @@
           comInstEventQueue("instruction-based event queue"),
           noSquashFromTC(false), trapPending(false), tc(nullptr)
     {
-        if (!FullSystem)
-            return;
-
-        if (cpu->params()->profile) {
-            profile = new FunctionProfile(
-                    m5::make_unique<TheISA::StackTrace>(),
-                    cpu->params()->system->workload->symtab(tc));
-            Callback *cb =
-                new MakeCallback<O3ThreadState,
-                &O3ThreadState::dumpFuncProfile>(this);
-            registerExitCallback(cb);
-        }
-
-        // let's fill with a dummy node for now so we don't get a segfault
-        // on the first cycle when there's no node available.
-        static ProfileNode dummyNode;
-        profileNode = &dummyNode;
-        profilePC = 3;
     }

     void serialize(CheckpointOut &cp) const override
@@ -152,14 +131,6 @@
     {
         process->syscall(tc, fault);
     }
-
-    void dumpFuncProfile()
-    {
-        OutputStream *os(
-            simout.create(csprintf("profile.%s.dat", cpu->name())));
-        profile->dump(*os->stream());
-        simout.close(os);
-    }
 };

 #endif // __CPU_O3_THREAD_STATE_HH__
diff --git a/src/cpu/simple/base.cc b/src/cpu/simple/base.cc
index 1dac921..c9db86c 100644
--- a/src/cpu/simple/base.cc
+++ b/src/cpu/simple/base.cc
@@ -41,7 +41,6 @@

 #include "cpu/simple/base.hh"

-#include "arch/stacktrace.hh"
 #include "arch/utility.hh"
 #include "base/cp_annotate.hh"
 #include "base/cprintf.hh"
@@ -57,7 +56,6 @@
 #include "cpu/checker/thread_context.hh"
 #include "cpu/exetrace.hh"
 #include "cpu/pred/bpred_unit.hh"
-#include "cpu/profile.hh"
 #include "cpu/simple/exec_context.hh"
 #include "cpu/simple_thread.hh"
 #include "cpu/smt.hh"
@@ -573,20 +571,11 @@
 BaseSimpleCPU::postExecute()
 {
     SimpleExecContext &t_info = *threadInfo[curThread];
-    SimpleThread* thread = t_info.thread;

     assert(curStaticInst);

     TheISA::PCState pc = threadContexts[curThread]->pcState();
     Addr instAddr = pc.instAddr();
-    if (FullSystem && thread->profile) {
-        bool usermode = TheISA::inUserMode(threadContexts[curThread]);
-        thread->profilePC = usermode ? 1 : instAddr;
- ProfileNode *node = thread->profile->consume(threadContexts[curThread],
-                                                     curStaticInst);
-        if (node)
-            thread->profileNode = node;
-    }

     if (curStaticInst->isMemRef()) {
         t_info.numMemRefs++;
diff --git a/src/cpu/simple_thread.cc b/src/cpu/simple_thread.cc
index 4efe6c1..1d2c0b8 100644
--- a/src/cpu/simple_thread.cc
+++ b/src/cpu/simple_thread.cc
@@ -43,7 +43,6 @@
 #include <string>

 #include "arch/isa_traits.hh"
-#include "arch/stacktrace.hh"
 #include "arch/utility.hh"
 #include "base/callback.hh"
 #include "base/compiler.hh"
@@ -52,7 +51,6 @@
 #include "base/trace.hh"
 #include "config/the_isa.hh"
 #include "cpu/base.hh"
-#include "cpu/profile.hh"
 #include "cpu/thread_context.hh"
 #include "mem/se_translating_port_proxy.hh"
 #include "mem/translating_port_proxy.hh"
@@ -91,21 +89,6 @@
     assert(isa);

     clearArchRegs();
-
-    if (baseCpu->params()->profile) {
- profile = new FunctionProfile(m5::make_unique<TheISA::StackTrace>(),
-                                      system->workload->symtab(this));
-        Callback *cb =
-            new MakeCallback<SimpleThread,
-            &SimpleThread::dumpFuncProfile>(this);
-        registerExitCallback(cb);
-    }
-
-    // let's fill with a dummy node for now so we don't get a segfault
-    // on the first cycle when there's no node available.
-    static ProfileNode dummyNode;
-    profileNode = &dummyNode;
-    profilePC = 3;
 }

 void
@@ -149,14 +132,6 @@
 }

 void
-SimpleThread::dumpFuncProfile()
-{
- OutputStream *os(simout.create(csprintf("profile.%s.dat", baseCpu->name())));
-    profile->dump(*os->stream());
-    simout.close(os);
-}
-
-void
 SimpleThread::activate()
 {
     if (status() == ThreadContext::Active)
diff --git a/src/cpu/simple_thread.hh b/src/cpu/simple_thread.hh
index d62fa0e..612174c 100644
--- a/src/cpu/simple_thread.hh
+++ b/src/cpu/simple_thread.hh
@@ -71,9 +71,6 @@
 class BaseCPU;
 class CheckerCPU;

-class FunctionProfile;
-class ProfileNode;
-
 /**
  * The SimpleThread object provides a combination of the ThreadState
  * object and the ThreadContext interface. It implements the
@@ -179,8 +176,6 @@
         dtb->demapPage(vaddr, asn);
     }

-    void dumpFuncProfile() override;
-
     /*******************************************
      * ThreadContext interface functions.
      ******************************************/
@@ -260,9 +255,6 @@
         return ThreadState::readLastSuspend();
     }

-    void profileClear() override { ThreadState::profileClear(); }
-    void profileSample() override { ThreadState::profileSample(); }
-
     void copyArchRegs(ThreadContext *tc) override;

     void
diff --git a/src/cpu/thread_context.hh b/src/cpu/thread_context.hh
index 6ebb191..8ca2c50 100644
--- a/src/cpu/thread_context.hh
+++ b/src/cpu/thread_context.hh
@@ -177,8 +177,6 @@
     /// Quiesce, suspend, and schedule activate at resume
     void quiesceTick(Tick resume);

-    virtual void dumpFuncProfile() = 0;
-
     virtual void takeOverFrom(ThreadContext *old_context) = 0;

     virtual void regStats(const std::string &name) {};
@@ -192,9 +190,6 @@
     virtual Tick readLastActivate() = 0;
     virtual Tick readLastSuspend() = 0;

-    virtual void profileClear() = 0;
-    virtual void profileSample() = 0;
-
     virtual void copyArchRegs(ThreadContext *tc) = 0;

     virtual void clearArchRegs() = 0;
diff --git a/src/cpu/thread_state.cc b/src/cpu/thread_state.cc
index f4859f6..f681abc 100644
--- a/src/cpu/thread_state.cc
+++ b/src/cpu/thread_state.cc
@@ -30,7 +30,6 @@

 #include "base/output.hh"
 #include "cpu/base.hh"
-#include "cpu/profile.hh"
 #include "mem/port.hh"
 #include "mem/port_proxy.hh"
 #include "mem/se_translating_port_proxy.hh"
@@ -43,7 +42,6 @@
     : numInst(0), numOp(0), numLoad(0), startNumLoad(0),
       _status(ThreadContext::Halted), baseCpu(cpu),
       _contextId(0), _threadId(_tid), lastActivate(0), lastSuspend(0),
-      profile(NULL), profileNode(NULL), profilePC(0),
       process(_process), physProxy(NULL), virtProxy(NULL),
       funcExeInst(0), storeCondFailures(0)
 {
@@ -118,17 +116,3 @@
     assert(virtProxy != NULL);
     return *virtProxy;
 }
-
-void
-ThreadState::profileClear()
-{
-    if (profile)
-        profile->clear();
-}
-
-void
-ThreadState::profileSample()
-{
-    if (profile)
-        profile->sample(profileNode, profilePC);
-}
diff --git a/src/cpu/thread_state.hh b/src/cpu/thread_state.hh
index e3f68b1..1cc92a1 100644
--- a/src/cpu/thread_state.hh
+++ b/src/cpu/thread_state.hh
@@ -32,13 +32,9 @@
 #include "arch/types.hh"
 #include "config/the_isa.hh"
 #include "cpu/base.hh"
-#include "cpu/profile.hh"
 #include "cpu/thread_context.hh"
 #include "sim/process.hh"

-class FunctionProfile;
-class ProfileNode;
-
 class Checkpoint;

 /**
@@ -82,12 +78,6 @@
      */
     void initMemProxies(ThreadContext *tc);

-    void dumpFuncProfile();
-
-    void profileClear();
-
-    void profileSample();
-
     PortProxy &getPhysProxy();

     PortProxy &getVirtProxy();
@@ -152,11 +142,6 @@
     /** Last time suspend was called on this thread. */
     Tick lastSuspend;

-  public:
-    FunctionProfile *profile;
-    ProfileNode *profileNode;
-    Addr profilePC;
-
   protected:
     Process *process;


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

Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I2a3319c1d5ad0ef8c99f5d35953b93c51b2a8a0b
Gerrit-Change-Number: 32214
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black <gabebl...@google.com>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

Reply via email to