Gabe Black has submitted this change. (
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
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/39335
Maintainer: Gabe Black <[email protected]>
Tested-by: kokoro <[email protected]>
Reviewed-by: Giacomo Travaglini <[email protected]>
Reviewed-by: Alex Dutu <[email protected]>
---
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(-)
Approvals:
Alex Dutu: Looks good to me, but someone else must approve
Giacomo Travaglini: Looks good to me, approved
Gabe Black: Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/arch/arm/utility.hh b/src/arch/arm/utility.hh
index 5b3b849..3c8acb2 100644
--- a/src/arch/arm/utility.hh
+++ b/src/arch/arm/utility.hh
@@ -375,12 +375,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 8c273f2..4b25810 100644
--- a/src/arch/riscv/faults.cc
+++ b/src/arch/riscv/faults.cc
@@ -141,7 +141,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 f179c17..87626b7 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 a5a842a..87049f0 100644
--- a/src/cpu/base_dyn_inst.hh
+++ b/src/cpu/base_dyn_inst.hh
@@ -590,7 +590,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 7dc62e0..66d1859 100644
--- a/src/cpu/checker/cpu_impl.hh
+++ b/src/cpu/checker/cpu_impl.hh
@@ -72,7 +72,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 7575cf9..a7f66e2 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"
@@ -239,9 +238,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 c8901bd..d5a6619 100644
--- a/src/cpu/minor/fetch2.cc
+++ b/src/cpu/minor/fetch2.cc
@@ -455,7 +455,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 4eae365..a435f3e 100644
--- a/src/cpu/o3/commit_impl.hh
+++ b/src/cpu/o3/commit_impl.hh
@@ -1109,7 +1109,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 7c54509..ab59fe1 100644
--- a/src/cpu/o3/fetch_impl.hh
+++ b/src/cpu/o3/fetch_impl.hh
@@ -527,7 +527,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 fe50055..ee2d0e7 100644
--- a/src/cpu/o3/iew_impl.hh
+++ b/src/cpu/o3/iew_impl.hh
@@ -460,7 +460,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 b25d115..a8d2592 100644
--- a/src/cpu/pred/bpred_unit.cc
+++ b/src/cpu/pred/bpred_unit.cc
@@ -227,7 +227,7 @@
RAS[tid].pop();
predict_record.pushedRAS = false;
}
- TheISA::advancePC(target, inst);
+ inst->advancePC(target);
}
} else {
predict_record.wasIndirect = true;
@@ -256,7 +256,7 @@
RAS[tid].pop();
predict_record.pushedRAS = false;
}
- TheISA::advancePC(target, inst);
+ inst->advancePC(target);
}
iPred->recordIndirect(pc.instAddr(), target.instAddr(),
seqNum,
tid);
@@ -266,7 +266,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 0941388..fe3e34d 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"
@@ -481,7 +480,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: 10
Gerrit-Owner: Gabe Black <[email protected]>
Gerrit-Reviewer: Alex Dutu <[email protected]>
Gerrit-Reviewer: Gabe Black <[email protected]>
Gerrit-Reviewer: Giacomo Travaglini <[email protected]>
Gerrit-Reviewer: Jason Lowe-Power <[email protected]>
Gerrit-Reviewer: kokoro <[email protected]>
Gerrit-MessageType: merged
_______________________________________________
gem5-dev mailing list -- [email protected]
To unsubscribe send an email to [email protected]
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
