From 44af887b2e2669f9841bdfc5ebb9b21a3b7515bb Mon Sep 17 00:00:00 2001 From: Gabe Black Date: Thu, 28 Jan 2021 18:56:45 -0800 Subject: [PATCH] arch,cpu: Move machInst into the arch specific StaticInst classes. This type is ISA specific. By moving it into the subclasses, it's still available to everybody that needs it but avoids that ISA dependence in the base StaticInst class. Change-Id: I87ac4c6eded42287ef9ebaa4c4a5738482a2fc13 Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/40101 Reviewed-by: Giacomo Travaglini Reviewed-by: Daniel Carvalho Maintainer: Giacomo Travaglini Tested-by: kokoro --- src/arch/arm/insts/static_inst.hh | 4 +++- src/arch/mips/isa/base.isa | 5 +++-- src/arch/power/insts/static_inst.hh | 5 +++-- src/arch/riscv/faults.cc | 7 +++++-- src/arch/riscv/insts/static_inst.hh | 7 ++++++- src/arch/sparc/insts/static_inst.hh | 7 ++++++- src/arch/x86/faults.cc | 4 +++- src/arch/x86/insts/static_inst.hh | 12 +++++++----- src/cpu/static_inst.cc | 7 +------ src/cpu/static_inst.hh | 8 ++------ 10 files changed, 39 insertions(+), 27 deletions(-) diff --git a/src/arch/arm/insts/static_inst.hh b/src/arch/arm/insts/static_inst.hh index 896523e3c1..b78f64a8d7 100644 --- a/src/arch/arm/insts/static_inst.hh +++ b/src/arch/arm/insts/static_inst.hh @@ -143,10 +143,12 @@ class ArmStaticInst : public StaticInst } } + ExtMachInst machInst; + // Constructor ArmStaticInst(const char *mnem, ExtMachInst _machInst, OpClass __opClass) - : StaticInst(mnem, _machInst, __opClass) + : StaticInst(mnem, __opClass), machInst(_machInst) { aarch64 = machInst.aarch64; if (bits(machInst, 28, 24) == 0x10) diff --git a/src/arch/mips/isa/base.isa b/src/arch/mips/isa/base.isa index cdd950c69c..0175bccfc9 100644 --- a/src/arch/mips/isa/base.isa +++ b/src/arch/mips/isa/base.isa @@ -42,10 +42,9 @@ output header {{ class MipsStaticInst : public StaticInst { protected: - // Constructor MipsStaticInst(const char *mnem, MachInst _machInst, OpClass __opClass) - : StaticInst(mnem, _machInst, __opClass) + : StaticInst(mnem, __opClass), machInst(_machInst) { } @@ -57,6 +56,8 @@ output header {{ Addr pc, const Loader::SymbolTable *symtab) const override; public: + ExtMachInst machInst; + void advancePC(MipsISA::PCState &pc) const override { diff --git a/src/arch/power/insts/static_inst.hh b/src/arch/power/insts/static_inst.hh index dc53bc1303..403d358e23 100644 --- a/src/arch/power/insts/static_inst.hh +++ b/src/arch/power/insts/static_inst.hh @@ -38,10 +38,11 @@ namespace PowerISA class PowerStaticInst : public StaticInst { protected: + ExtMachInst machInst; // Constructor - PowerStaticInst(const char *mnem, MachInst _machInst, OpClass __opClass) - : StaticInst(mnem, _machInst, __opClass) + PowerStaticInst(const char *mnem, ExtMachInst _machInst, OpClass __opClass) + : StaticInst(mnem, __opClass), machInst(_machInst) { } diff --git a/src/arch/riscv/faults.cc b/src/arch/riscv/faults.cc index 5ac2a3c790..8c273f2a98 100644 --- a/src/arch/riscv/faults.cc +++ b/src/arch/riscv/faults.cc @@ -32,6 +32,7 @@ #include "arch/riscv/faults.hh" #include "arch/riscv/fs_workload.hh" +#include "arch/riscv/insts/static_inst.hh" #include "arch/riscv/isa.hh" #include "arch/riscv/registers.hh" #include "arch/riscv/utility.hh" @@ -163,14 +164,16 @@ void Reset::invoke(ThreadContext *tc, const StaticInstPtr &inst) void UnknownInstFault::invokeSE(ThreadContext *tc, const StaticInstPtr &inst) { - panic("Unknown instruction 0x%08x at pc 0x%016llx", inst->machInst, + auto *rsi = static_cast(inst.get()); + panic("Unknown instruction 0x%08x at pc 0x%016llx", rsi->machInst, tc->pcState().pc()); } void IllegalInstFault::invokeSE(ThreadContext *tc, const StaticInstPtr &inst) { - panic("Illegal instruction 0x%08x at pc 0x%016llx: %s", inst->machInst, + auto *rsi = static_cast(inst.get()); + panic("Illegal instruction 0x%08x at pc 0x%016llx: %s", rsi->machInst, tc->pcState().pc(), reason.c_str()); } diff --git a/src/arch/riscv/insts/static_inst.hh b/src/arch/riscv/insts/static_inst.hh index 149e847a31..1c57cc7f4e 100644 --- a/src/arch/riscv/insts/static_inst.hh +++ b/src/arch/riscv/insts/static_inst.hh @@ -46,9 +46,14 @@ namespace RiscvISA class RiscvStaticInst : public StaticInst { protected: - using StaticInst::StaticInst; + RiscvStaticInst(const char *_mnemonic, ExtMachInst _machInst, + OpClass __opClass) : + StaticInst(_mnemonic, __opClass), machInst(_machInst) + {} public: + ExtMachInst machInst; + void advancePC(PCState &pc) const override { pc.advance(); } size_t diff --git a/src/arch/sparc/insts/static_inst.hh b/src/arch/sparc/insts/static_inst.hh index 0237c986ed..43d8d3d86a 100644 --- a/src/arch/sparc/insts/static_inst.hh +++ b/src/arch/sparc/insts/static_inst.hh @@ -87,7 +87,12 @@ enum FpCondTest class SparcStaticInst : public StaticInst { protected: - using StaticInst::StaticInst; + ExtMachInst machInst; + + SparcStaticInst(const char *_mnemonic, ExtMachInst _machInst, + OpClass __opClass) : + StaticInst(_mnemonic, __opClass), machInst(_machInst) + {} std::string generateDisassembly( Addr pc, const Loader::SymbolTable *symtab) const override; diff --git a/src/arch/x86/faults.cc b/src/arch/x86/faults.cc index a50751510d..c53babfbf7 100644 --- a/src/arch/x86/faults.cc +++ b/src/arch/x86/faults.cc @@ -41,6 +41,7 @@ #include "arch/x86/faults.hh" #include "arch/x86/generated/decoder.hh" +#include "arch/x86/insts/static_inst.hh" #include "arch/x86/isa_traits.hh" #include "arch/x86/mmu.hh" #include "base/loader/symtab.hh" @@ -128,8 +129,9 @@ InvalidOpcode::invoke(ThreadContext *tc, const StaticInstPtr &inst) if (FullSystem) { X86Fault::invoke(tc, inst); } else { + auto *xsi = static_cast(inst.get()); panic("Unrecognized/invalid instruction executed:\n %s", - inst->machInst); + xsi->machInst); } } diff --git a/src/arch/x86/insts/static_inst.hh b/src/arch/x86/insts/static_inst.hh index b4e8f0f3b1..64c56eac7f 100644 --- a/src/arch/x86/insts/static_inst.hh +++ b/src/arch/x86/insts/static_inst.hh @@ -84,12 +84,14 @@ class X86StaticInst : public StaticInst protected: using ExtMachInst = X86ISA::ExtMachInst; + public: + ExtMachInst machInst; + + protected: // Constructor. - X86StaticInst(const char *mnem, - ExtMachInst _machInst, OpClass __opClass) - : StaticInst(mnem, _machInst, __opClass) - { - } + X86StaticInst(const char *mnem, ExtMachInst _machInst, OpClass __opClass) : + StaticInst(mnem, __opClass), machInst(_machInst) + {} std::string generateDisassembly( Addr pc, const Loader::SymbolTable *symtab) const override; diff --git a/src/cpu/static_inst.cc b/src/cpu/static_inst.cc index 86cc4c2bde..05cd36067a 100644 --- a/src/cpu/static_inst.cc +++ b/src/cpu/static_inst.cc @@ -34,13 +34,10 @@ namespace { -static TheISA::ExtMachInst nopMachInst; - class NopStaticInst : public StaticInst { public: - NopStaticInst() : StaticInst("gem5 nop", nopMachInst, No_OpClass) - {} + NopStaticInst() : StaticInst("gem5 nop", No_OpClass) {} Fault execute(ExecContext *xc, Trace::InstRecord *traceData) const override @@ -60,8 +57,6 @@ class NopStaticInst : public StaticInst { return mnemonic; } - - private: }; } diff --git a/src/cpu/static_inst.hh b/src/cpu/static_inst.hh index b2cd50851b..b01471938f 100644 --- a/src/cpu/static_inst.hh +++ b/src/cpu/static_inst.hh @@ -255,9 +255,6 @@ class StaticInst : public RefCounted, public StaticInstFlags /// Pointer to a statically allocated generic "nop" instruction object. static StaticInstPtr nopStaticInstPtr; - /// The binary machine instruction. - const TheISA::ExtMachInst machInst; - virtual uint64_t getEMI() const { return 0; } protected: @@ -300,12 +297,11 @@ class StaticInst : public RefCounted, public StaticInstFlags /// default, since the decoder generally only overrides /// the fields that are meaningful for the particular /// instruction. - StaticInst(const char *_mnemonic, TheISA::ExtMachInst _machInst, - OpClass __opClass) + StaticInst(const char *_mnemonic, OpClass __opClass) : _opClass(__opClass), _numSrcRegs(0), _numDestRegs(0), _numFPDestRegs(0), _numIntDestRegs(0), _numCCDestRegs(0), _numVecDestRegs(0), - _numVecElemDestRegs(0), _numVecPredDestRegs(0), machInst(_machInst), + _numVecElemDestRegs(0), _numVecPredDestRegs(0), mnemonic(_mnemonic), cachedDisassembly(0) { }