Gabe Black has uploaded this change for review. ( 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
---
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(-)



diff --git a/src/arch/arm/insts/static_inst.hh b/src/arch/arm/insts/static_inst.hh
index 908641c..1f4251a 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)
     { }


--
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: 1
Gerrit-Owner: Gabe Black <gabe.bl...@gmail.com>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

Reply via email to