From 4b361aa4bc6315ac33209b710338988de8593f58 Mon Sep 17 00:00:00 2001 From: Gabe Black Date: Thu, 7 Oct 2021 18:14:55 -0700 Subject: [PATCH] arch,cpu: Refactor PCState construction a little. Make the Addr constructor explicit to avoid implicit/hidden conversions from Addr. Also, add a copy constructor to the PCState types, and explicitly enable the assignment operator. Change-Id: Ibef17ece7fd06b2f9709c46d118e88a80da0b194 Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/52036 Tested-by: kokoro Reviewed-by: Daniel Carvalho Maintainer: Bobby R. Bruce --- src/arch/arm/faults.cc | 2 +- src/arch/arm/isa.cc | 2 +- src/arch/arm/pcstate.hh | 13 ++++++++++--- src/arch/generic/pcstate.hh | 24 ++++++++++++++++++++---- src/arch/power/insts/branch.cc | 6 +++--- src/arch/power/pcstate.hh | 6 ++++++ src/arch/riscv/faults.cc | 5 +++-- src/arch/riscv/isa/formats/standard.isa | 2 +- src/arch/x86/pcstate.hh | 6 ++++-- src/cpu/o3/fetch.cc | 2 +- src/cpu/pred/btb.cc | 2 +- src/cpu/simple_thread.hh | 2 +- src/cpu/thread_context.hh | 1 + 13 files changed, 53 insertions(+), 20 deletions(-) diff --git a/src/arch/arm/faults.cc b/src/arch/arm/faults.cc index 1e94329cf5..b8ca6d289c 100644 --- a/src/arch/arm/faults.cc +++ b/src/arch/arm/faults.cc @@ -807,7 +807,7 @@ Reset::invoke(ThreadContext *tc, const StaticInstPtr &inst) } } else { // Advance the PC to the IMPLEMENTATION DEFINED reset value - PCState pc = ArmSystem::resetAddr(tc); + PCState pc(ArmSystem::resetAddr(tc)); pc.aarch64(true); pc.nextAArch64(true); tc->pcState(pc); diff --git a/src/arch/arm/isa.cc b/src/arch/arm/isa.cc index a1e759c3ba..7e047f1715 100644 --- a/src/arch/arm/isa.cc +++ b/src/arch/arm/isa.cc @@ -607,7 +607,7 @@ RegVal ISA::readMiscReg(int misc_reg) { CPSR cpsr = 0; - PCState pc = 0; + PCState pc(0); SCR scr = 0; if (misc_reg == MISCREG_CPSR) { diff --git a/src/arch/arm/pcstate.hh b/src/arch/arm/pcstate.hh index 5bbad266f4..d2f3cefff2 100644 --- a/src/arch/arm/pcstate.hh +++ b/src/arch/arm/pcstate.hh @@ -91,8 +91,6 @@ class PCState : public GenericISA::UPCState<4> bool _stepped = false; public: - PCState() {} - void set(Addr val) { @@ -100,7 +98,16 @@ class PCState : public GenericISA::UPCState<4> npc(val + (thumb() ? 2 : 4)); } - PCState(Addr val) { set(val); } + PCState(const PCState &other) : Base(other), + flags(other.flags), nextFlags(other.nextFlags), + _itstate(other._itstate), _nextItstate(other._nextItstate), + _size(other._size), _illegalExec(other._illegalExec), + _debugStep(other._debugStep), _stepped(other._stepped) + {} + PCState &operator=(const PCState &other) = default; + + PCState() {} + explicit PCState(Addr val) { set(val); } PCStateBase *clone() const override { return new PCState(*this); } diff --git a/src/arch/generic/pcstate.hh b/src/arch/generic/pcstate.hh index 6d4e23e433..762b7680de 100644 --- a/src/arch/generic/pcstate.hh +++ b/src/arch/generic/pcstate.hh @@ -85,6 +85,10 @@ class PCStateCommon : public PCStateBase MicroPC _upc = 0; MicroPC _nupc = 1; + PCStateCommon(const PCStateCommon &other) : + _pc(other._pc), _npc(other._npc), _upc(other._upc), _nupc(other._nupc) + {} + PCStateCommon &operator=(const PCStateCommon &other) = default; PCStateCommon() {} public: @@ -187,8 +191,10 @@ class SimplePCState : public PCStateCommon typedef PCStateCommon Base; public: + SimplePCState(const SimplePCState &other) : Base(other) {} + SimplePCState &operator=(const SimplePCState &other) = default; SimplePCState() {} - SimplePCState(Addr val) { set(val); } + explicit SimplePCState(Addr val) { set(val); } PCStateBase * clone() const override @@ -260,8 +266,10 @@ class UPCState : public SimplePCState nupc(1); } + UPCState(const UPCState &other) : Base(other) {} + UPCState &operator=(const UPCState &other) = default; UPCState() {} - UPCState(Addr val) { set(val); } + explicit UPCState(Addr val) { set(val); } bool branching() const @@ -336,8 +344,12 @@ class DelaySlotPCState : public SimplePCState nnpc(val + 2 * InstWidth); } + DelaySlotPCState(const DelaySlotPCState &other) : + Base(other), _nnpc(other._nnpc) + {} + DelaySlotPCState &operator=(const DelaySlotPCState &other) = default; DelaySlotPCState() {} - DelaySlotPCState(Addr val) { set(val); } + explicit DelaySlotPCState(Addr val) { set(val); } bool branching() const @@ -431,8 +443,12 @@ class DelaySlotUPCState : public DelaySlotPCState nupc(1); } + DelaySlotUPCState(const DelaySlotUPCState &other) : + Base(other), _upc(other._upc), _nupc(other._nupc) + {} + DelaySlotUPCState &operator=(const DelaySlotUPCState &other) = default; DelaySlotUPCState() {} - DelaySlotUPCState(Addr val) { set(val); } + explicit DelaySlotUPCState(Addr val) { set(val); } bool branching() const diff --git a/src/arch/power/insts/branch.cc b/src/arch/power/insts/branch.cc index 899214388e..861bacd43a 100644 --- a/src/arch/power/insts/branch.cc +++ b/src/arch/power/insts/branch.cc @@ -67,7 +67,7 @@ BranchOp::branchTarget(ThreadContext *tc) const else addr = tc->pcState().pc() + li; - return msr.sf ? addr : addr & UINT32_MAX; + return PowerISA::PCState(msr.sf ? addr : addr & UINT32_MAX); } @@ -115,7 +115,7 @@ BranchDispCondOp::branchTarget(ThreadContext *tc) const else addr = tc->pcState().pc() + bd; - return msr.sf ? addr : addr & UINT32_MAX; + return PowerISA::PCState(msr.sf ? addr : addr & UINT32_MAX); } @@ -160,7 +160,7 @@ BranchRegCondOp::branchTarget(ThreadContext *tc) const { Msr msr = tc->readIntReg(INTREG_MSR); Addr addr = tc->readIntReg(srcRegIdx(_numSrcRegs - 1).index()) & -4ULL; - return msr.sf ? addr : addr & UINT32_MAX; + return PowerISA::PCState(msr.sf ? addr : addr & UINT32_MAX); } diff --git a/src/arch/power/pcstate.hh b/src/arch/power/pcstate.hh index c46b37e6c7..2b5ddd3f17 100644 --- a/src/arch/power/pcstate.hh +++ b/src/arch/power/pcstate.hh @@ -47,6 +47,12 @@ class PCState : public GenericISA::SimplePCState<4> public: using GenericISA::SimplePCState<4>::SimplePCState; + PCState(const PCState &other) : + GenericISA::SimplePCState<4>(other), + guestByteOrder(other.guestByteOrder) + {} + PCState &operator=(const PCState &other) = default; + PCStateBase *clone() const override { return new PCState(*this); } ByteOrder diff --git a/src/arch/riscv/faults.cc b/src/arch/riscv/faults.cc index b7fb50938c..36e2ce9531 100644 --- a/src/arch/riscv/faults.cc +++ b/src/arch/riscv/faults.cc @@ -149,7 +149,8 @@ RiscvFault::invoke(ThreadContext *tc, const StaticInstPtr &inst) tc->pcState(pcState); } -void Reset::invoke(ThreadContext *tc, const StaticInstPtr &inst) +void +Reset::invoke(ThreadContext *tc, const StaticInstPtr &inst) { tc->setMiscReg(MISCREG_PRV, PRV_M); STATUS status = tc->readMiscReg(MISCREG_STATUS); @@ -160,7 +161,7 @@ void Reset::invoke(ThreadContext *tc, const StaticInstPtr &inst) // Advance the PC to the implementation-defined reset vector auto workload = dynamic_cast(tc->getSystemPtr()->workload); - PCState pc = workload->getEntry(); + PCState pc(workload->getEntry()); tc->pcState(pc); } diff --git a/src/arch/riscv/isa/formats/standard.isa b/src/arch/riscv/isa/formats/standard.isa index dad2c2baf0..ce58e54271 100644 --- a/src/arch/riscv/isa/formats/standard.isa +++ b/src/arch/riscv/isa/formats/standard.isa @@ -197,7 +197,7 @@ def template BranchExecute {{ RiscvISA::PCState %(class_name)s::branchTarget(const RiscvISA::PCState &branchPC) const { - return branchPC.pc() + imm; + return RiscvISA::PCState(branchPC.pc() + imm); } std::string diff --git a/src/arch/x86/pcstate.hh b/src/arch/x86/pcstate.hh index 28547f3593..c0641ca127 100644 --- a/src/arch/x86/pcstate.hh +++ b/src/arch/x86/pcstate.hh @@ -50,7 +50,7 @@ namespace X86ISA class PCState : public GenericISA::UPCState<8> { protected: - typedef GenericISA::UPCState<8> Base; + using Base = GenericISA::UPCState<8>; uint8_t _size; @@ -64,8 +64,10 @@ class PCState : public GenericISA::UPCState<8> _size = 0; } + PCState(const PCState &other) : Base(other), _size(other._size) {} + PCState &operator=(const PCState &other) = default; PCState() {} - PCState(Addr val) { set(val); } + explicit PCState(Addr val) { set(val); } void setNPC(Addr val) diff --git a/src/cpu/o3/fetch.cc b/src/cpu/o3/fetch.cc index 5bbdc0306e..cfc3a83e55 100644 --- a/src/cpu/o3/fetch.cc +++ b/src/cpu/o3/fetch.cc @@ -120,7 +120,7 @@ Fetch::Fetch(CPU *_cpu, const O3CPUParams ¶ms) for (int i = 0; i < MaxThreads; i++) { fetchStatus[i] = Idle; decoder[i] = nullptr; - pc[i] = 0; + pc[i].set(0); fetchOffset[i] = 0; macroop[i] = nullptr; delayedCommit[i] = false; diff --git a/src/cpu/pred/btb.cc b/src/cpu/pred/btb.cc index a88ce67f80..a3950cd0bd 100644 --- a/src/cpu/pred/btb.cc +++ b/src/cpu/pred/btb.cc @@ -126,7 +126,7 @@ DefaultBTB::lookup(Addr instPC, ThreadID tid) && btb[btb_idx].tid == tid) { return btb[btb_idx].target; } else { - return 0; + return TheISA::PCState(0); } } diff --git a/src/cpu/simple_thread.hh b/src/cpu/simple_thread.hh index 71e1c1f08d..570489146c 100644 --- a/src/cpu/simple_thread.hh +++ b/src/cpu/simple_thread.hh @@ -249,7 +249,7 @@ class SimpleThread : public ThreadState, public ThreadContext void clearArchRegs() override { - _pcState = 0; + _pcState.set(0); std::fill(intRegs.begin(), intRegs.end(), 0); std::fill(floatRegs.begin(), floatRegs.end(), 0); for (auto &vec_reg: vecRegs) diff --git a/src/cpu/thread_context.hh b/src/cpu/thread_context.hh index 3ae8a7f2a9..fee250356e 100644 --- a/src/cpu/thread_context.hh +++ b/src/cpu/thread_context.hh @@ -226,6 +226,7 @@ class ThreadContext : public PCEventScope virtual TheISA::PCState pcState() const = 0; virtual void pcState(const TheISA::PCState &val) = 0; + void pcState(Addr addr) { pcState(TheISA::PCState(addr)); } void setNPC(Addr val)