Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/39335 )
Change subject: arch,cpu: Collapse away TheISA::advancePC.
......................................................................
arch,cpu: Collapse away TheISA::advancePC.
In most ISAs except MIPS and Power, this was implemented as
inst->advancePC(). It works just fine to call this function all the
time, but the idea had originally been that for ISAs which could simply
advance the PC using the PC itself, they could save the virtual function
call. Since the only ISAs which could skip the call were MIPS and Power,
and neither is at the point where that level of performance tuning
matters, this function can be collapsed with little downside.
If this turns out to be a performance bottleneck in the future, the way
the PC is managed could be revisited to see if we can factor out this
trip to the instruction object in the first place.
Change-Id: I533d1ad316e5c936466c529b7f1238a9ab87bd1c
---
M src/arch/arm/utility.hh
M src/arch/mips/utility.hh
M src/arch/power/utility.hh
M src/arch/riscv/faults.cc
M src/arch/riscv/utility.hh
M src/arch/sparc/utility.hh
M src/arch/x86/utility.hh
M src/cpu/base_dyn_inst.hh
M src/cpu/checker/cpu_impl.hh
M src/cpu/minor/execute.cc
M src/cpu/minor/fetch2.cc
M src/cpu/o3/commit_impl.hh
M src/cpu/o3/fetch_impl.hh
M src/cpu/o3/iew_impl.hh
M src/cpu/pred/bpred_unit.cc
M src/cpu/simple/base.cc
16 files changed, 13 insertions(+), 54 deletions(-)
diff --git a/src/arch/arm/utility.hh b/src/arch/arm/utility.hh
index 853643b..52c6ba0 100644
--- a/src/arch/arm/utility.hh
+++ b/src/arch/arm/utility.hh
@@ -374,12 +374,6 @@
bool SPAlignmentCheckEnabled(ThreadContext* tc);
-inline void
-advancePC(PCState &pc, const StaticInstPtr &inst)
-{
- inst->advancePC(pc);
-}
-
Addr truncPage(Addr addr);
Addr roundPage(Addr addr);
diff --git a/src/arch/mips/utility.hh b/src/arch/mips/utility.hh
index 1804680..f86e93a 100644
--- a/src/arch/mips/utility.hh
+++ b/src/arch/mips/utility.hh
@@ -72,14 +72,6 @@
return (addr + PageBytes - 1) & ~(PageBytes - 1);
}
-void copyRegs(ThreadContext *src, ThreadContext *dest);
-
-inline void
-advancePC(PCState &pc, const StaticInstPtr &inst)
-{
- pc.advance();
-}
-
};
diff --git a/src/arch/power/utility.hh b/src/arch/power/utility.hh
index 03df7fb..13183f1 100644
--- a/src/arch/power/utility.hh
+++ b/src/arch/power/utility.hh
@@ -39,12 +39,6 @@
void copyRegs(ThreadContext *src, ThreadContext *dest);
-inline void
-advancePC(PCState &pc, const StaticInstPtr &inst)
-{
- pc.advance();
-}
-
} // namespace PowerISA
diff --git a/src/arch/riscv/faults.cc b/src/arch/riscv/faults.cc
index 5ac2a3c..1c14f0b 100644
--- a/src/arch/riscv/faults.cc
+++ b/src/arch/riscv/faults.cc
@@ -140,7 +140,7 @@
pcState.set(addr);
} else {
invokeSE(tc, inst);
- advancePC(pcState, inst);
+ inst->advancePC(pcState);
}
tc->pcState(pcState);
}
diff --git a/src/arch/riscv/utility.hh b/src/arch/riscv/utility.hh
index 3bbbd71..f033b2b 100644
--- a/src/arch/riscv/utility.hh
+++ b/src/arch/riscv/utility.hh
@@ -142,12 +142,6 @@
}
}
-inline void
-advancePC(PCState &pc, const StaticInstPtr &inst)
-{
- inst->advancePC(pc);
-}
-
} // namespace RiscvISA
#endif // __ARCH_RISCV_UTILITY_HH__
diff --git a/src/arch/sparc/utility.hh b/src/arch/sparc/utility.hh
index 792d7c4..78e5e25 100644
--- a/src/arch/sparc/utility.hh
+++ b/src/arch/sparc/utility.hh
@@ -43,12 +43,6 @@
void copyRegs(ThreadContext *src, ThreadContext *dest);
-inline void
-advancePC(PCState &pc, const StaticInstPtr &inst)
-{
- inst->advancePC(pc);
-}
-
} // namespace SparcISA
#endif
diff --git a/src/arch/x86/utility.hh b/src/arch/x86/utility.hh
index 9cd4e94..a572637 100644
--- a/src/arch/x86/utility.hh
+++ b/src/arch/x86/utility.hh
@@ -46,12 +46,6 @@
{
void copyRegs(ThreadContext *src, ThreadContext *dest);
- inline void
- advancePC(PCState &pc, const StaticInstPtr &inst)
- {
- inst->advancePC(pc);
- }
-
/**
* Reconstruct the rflags register from the internal gem5 register
* state.
diff --git a/src/cpu/base_dyn_inst.hh b/src/cpu/base_dyn_inst.hh
index 0b8a272..61230fc 100644
--- a/src/cpu/base_dyn_inst.hh
+++ b/src/cpu/base_dyn_inst.hh
@@ -504,7 +504,7 @@
mispredicted()
{
TheISA::PCState tempPC = pc;
- TheISA::advancePC(tempPC, staticInst);
+ staticInst->advancePC(tempPC);
return !(tempPC == predPC);
}
diff --git a/src/cpu/checker/cpu_impl.hh b/src/cpu/checker/cpu_impl.hh
index 70dc451..8839903 100644
--- a/src/cpu/checker/cpu_impl.hh
+++ b/src/cpu/checker/cpu_impl.hh
@@ -75,7 +75,7 @@
if (curStaticInst->isLastMicroop())
curMacroStaticInst = StaticInst::nullStaticInstPtr;
TheISA::PCState pcState = thread->pcState();
- TheISA::advancePC(pcState, curStaticInst);
+ curStaticInst->advancePC(pcState);
thread->pcState(pcState);
DPRINTF(Checker, "Advancing PC to %s.\n", thread->pcState());
}
diff --git a/src/cpu/minor/execute.cc b/src/cpu/minor/execute.cc
index 3eb7811..7672c0c 100644
--- a/src/cpu/minor/execute.cc
+++ b/src/cpu/minor/execute.cc
@@ -39,7 +39,6 @@
#include "arch/locked_mem.hh"
#include "arch/registers.hh"
-#include "arch/utility.hh"
#include "cpu/minor/cpu.hh"
#include "cpu/minor/exec_context.hh"
#include "cpu/minor/fetch1.hh"
@@ -237,9 +236,8 @@
/* The reason for the branch data we're about to generate, set below */
BranchData::Reason reason = BranchData::NoBranch;
- if (fault == NoFault)
- {
- TheISA::advancePC(target, inst->staticInst);
+ if (fault == NoFault) {
+ inst->staticInst->advancePC(target);
thread->pcState(target);
DPRINTF(Branch, "Advancing current PC from: %s to: %s\n",
diff --git a/src/cpu/minor/fetch2.cc b/src/cpu/minor/fetch2.cc
index 08c280a..7cbd77c 100644
--- a/src/cpu/minor/fetch2.cc
+++ b/src/cpu/minor/fetch2.cc
@@ -454,7 +454,7 @@
#endif
/* Advance PC for the next instruction */
- TheISA::advancePC(fetch_info.pc, decoded_inst);
+ decoded_inst->advancePC(fetch_info.pc);
/* Predict any branches and issue a branch if
* necessary */
diff --git a/src/cpu/o3/commit_impl.hh b/src/cpu/o3/commit_impl.hh
index 717ae33..8808e74 100644
--- a/src/cpu/o3/commit_impl.hh
+++ b/src/cpu/o3/commit_impl.hh
@@ -1104,7 +1104,7 @@
cpu->traceFunctions(pc[tid].instAddr());
- TheISA::advancePC(pc[tid], head_inst->staticInst);
+ head_inst->staticInst->advancePC(pc[tid]);
// Keep track of the last sequence number commited
lastCommitedSeqNum[tid] = head_inst->seqNum;
diff --git a/src/cpu/o3/fetch_impl.hh b/src/cpu/o3/fetch_impl.hh
index 6f03e78..9232d1a 100644
--- a/src/cpu/o3/fetch_impl.hh
+++ b/src/cpu/o3/fetch_impl.hh
@@ -525,7 +525,7 @@
bool predict_taken;
if (!inst->isControl()) {
- TheISA::advancePC(nextPC, inst->staticInst);
+ inst->staticInst->advancePC(nextPC);
inst->setPredTarg(nextPC);
inst->setPredTaken(false);
return false;
diff --git a/src/cpu/o3/iew_impl.hh b/src/cpu/o3/iew_impl.hh
index 9e56ed0..f7629b2 100644
--- a/src/cpu/o3/iew_impl.hh
+++ b/src/cpu/o3/iew_impl.hh
@@ -452,7 +452,7 @@
toCommit->branchTaken[tid] = inst->pcState().branching();
TheISA::PCState pc = inst->pcState();
- TheISA::advancePC(pc, inst->staticInst);
+ inst->staticInst->advancePC(pc);
toCommit->pc[tid] = pc;
toCommit->mispredictInst[tid] = inst;
diff --git a/src/cpu/pred/bpred_unit.cc b/src/cpu/pred/bpred_unit.cc
index 5d6c969..b5e00c0 100644
--- a/src/cpu/pred/bpred_unit.cc
+++ b/src/cpu/pred/bpred_unit.cc
@@ -223,7 +223,7 @@
RAS[tid].pop();
predict_record.pushedRAS = false;
}
- TheISA::advancePC(target, inst);
+ inst->advancePC(target);
}
} else {
predict_record.wasIndirect = true;
@@ -252,7 +252,7 @@
RAS[tid].pop();
predict_record.pushedRAS = false;
}
- TheISA::advancePC(target, inst);
+ inst->advancePC(target);
}
iPred->recordIndirect(pc.instAddr(), target.instAddr(),
seqNum,
tid);
@@ -262,7 +262,7 @@
if (inst->isReturn()) {
predict_record.wasReturn = true;
}
- TheISA::advancePC(target, inst);
+ inst->advancePC(target);
}
predict_record.target = target.instAddr();
diff --git a/src/cpu/simple/base.cc b/src/cpu/simple/base.cc
index 7f4797b..42c8aad 100644
--- a/src/cpu/simple/base.cc
+++ b/src/cpu/simple/base.cc
@@ -41,7 +41,6 @@
#include "cpu/simple/base.hh"
-#include "arch/utility.hh"
#include "base/cprintf.hh"
#include "base/inifile.hh"
#include "base/loader/symtab.hh"
@@ -487,7 +486,7 @@
if (curStaticInst->isLastMicroop())
curMacroStaticInst = StaticInst::nullStaticInstPtr;
TheISA::PCState pcState = thread->pcState();
- TheISA::advancePC(pcState, curStaticInst);
+ curStaticInst->advancePC(pcState);
thread->pcState(pcState);
}
}
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/39335
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: I533d1ad316e5c936466c529b7f1238a9ab87bd1c
Gerrit-Change-Number: 39335
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black <[email protected]>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list -- [email protected]
To unsubscribe send an email to [email protected]
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
