Gabe Black has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/40101 )
Change subject: arch,cpu: Move machInst into the arch specific StaticInst
classes.
......................................................................
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 <[email protected]>
Reviewed-by: Daniel Carvalho <[email protected]>
Maintainer: Giacomo Travaglini <[email protected]>
Tested-by: kokoro <[email protected]>
---
M src/arch/arm/insts/static_inst.hh
M src/arch/mips/isa/base.isa
M src/arch/power/insts/static_inst.hh
M src/arch/riscv/faults.cc
M src/arch/riscv/insts/static_inst.hh
M src/arch/sparc/insts/static_inst.hh
M src/arch/x86/faults.cc
M src/arch/x86/insts/static_inst.hh
M src/cpu/static_inst.cc
M src/cpu/static_inst.hh
10 files changed, 39 insertions(+), 27 deletions(-)
Approvals:
Giacomo Travaglini: Looks good to me, approved; Looks good to me, approved
Daniel Carvalho: Looks good to me, but someone else must approve
kokoro: Regressions pass
diff --git a/src/arch/arm/insts/static_inst.hh
b/src/arch/arm/insts/static_inst.hh
index 896523e..b78f64a 100644
--- a/src/arch/arm/insts/static_inst.hh
+++ b/src/arch/arm/insts/static_inst.hh
@@ -143,10 +143,12 @@
}
}
+ 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 cdd950c..0175bcc 100644
--- a/src/arch/mips/isa/base.isa
+++ b/src/arch/mips/isa/base.isa
@@ -42,10 +42,9 @@
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 @@
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 dc53bc1..403d358 100644
--- a/src/arch/power/insts/static_inst.hh
+++ b/src/arch/power/insts/static_inst.hh
@@ -38,10 +38,11 @@
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 5ac2a3c..8c273f2 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
UnknownInstFault::invokeSE(ThreadContext *tc, const StaticInstPtr &inst)
{
- panic("Unknown instruction 0x%08x at pc 0x%016llx", inst->machInst,
+ auto *rsi = static_cast<RiscvStaticInst *>(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<RiscvStaticInst *>(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 149e847..1c57cc7 100644
--- a/src/arch/riscv/insts/static_inst.hh
+++ b/src/arch/riscv/insts/static_inst.hh
@@ -46,9 +46,14 @@
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 0237c98..43d8d3d 100644
--- a/src/arch/sparc/insts/static_inst.hh
+++ b/src/arch/sparc/insts/static_inst.hh
@@ -87,7 +87,12 @@
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 a507515..c53babf 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 @@
if (FullSystem) {
X86Fault::invoke(tc, inst);
} else {
+ auto *xsi = static_cast<X86StaticInst *>(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 b4e8f0f..64c56ea 100644
--- a/src/arch/x86/insts/static_inst.hh
+++ b/src/arch/x86/insts/static_inst.hh
@@ -84,12 +84,14 @@
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 86cc4c2..05cd360 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 @@
{
return mnemonic;
}
-
- private:
};
}
diff --git a/src/cpu/static_inst.hh b/src/cpu/static_inst.hh
index b2cd508..b014719 100644
--- a/src/cpu/static_inst.hh
+++ b/src/cpu/static_inst.hh
@@ -255,9 +255,6 @@
/// 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 @@
/// 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)
{ }
9 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the
submitted one.
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/40101
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: I87ac4c6eded42287ef9ebaa4c4a5738482a2fc13
Gerrit-Change-Number: 40101
Gerrit-PatchSet: 11
Gerrit-Owner: Gabe Black <[email protected]>
Gerrit-Reviewer: Andreas Sandberg <[email protected]>
Gerrit-Reviewer: Brandon Potter <[email protected]>
Gerrit-Reviewer: Daniel Carvalho <[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
