Gabe Black has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/52043 )

 (

4 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
 )Change subject: arch,cpu,sim: Use PCStateBase in advancePC.
......................................................................

arch,cpu,sim: Use PCStateBase in advancePC.

By using a PCStateBase pointer or reference, we can (mostly) avoid
having to know what the ISA specific PState class is, letting the ISA
specific instruction classes cast to the type they need internally.

There are a couple minor places where we need to do those casts outside
of ISA specific types, one in the generic NopStaticInstPtr class, and a
few in generic faults.

Right now, we'll just use the TheISA::PCState type in those isolated
spots (sometimes hidden by auto), and deal with it later, possibly
with a virtual "advance" method of some sort.

Change-Id: I774c67dc648a85556230f601e087211b3d5630a9
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/52043
Tested-by: kokoro <[email protected]>
Maintainer: Gabe Black <[email protected]>
Reviewed-by: Daniel Carvalho <[email protected]>
---
M src/arch/arm/insts/static_inst.hh
M src/arch/x86/insts/microop.hh
M src/arch/x86/insts/static_inst.hh
M src/arch/power/insts/static_inst.hh
M src/cpu/nop_static_inst.cc
M src/cpu/static_inst.hh
M src/arch/mips/isa/base.isa
M src/arch/arm/insts/macromem.hh
M src/arch/riscv/insts/static_inst.cc
M src/arch/riscv/insts/static_inst.hh
M src/arch/sparc/insts/micro.hh
M src/arch/arm/insts/mem.hh
M src/arch/arm/insts/mem64.hh
M src/arch/arm/insts/pred_inst.hh
M src/arch/arm/insts/vfp.hh
M src/arch/sparc/insts/static_inst.cc
M src/arch/sparc/insts/static_inst.hh
17 files changed, 100 insertions(+), 48 deletions(-)

Approvals:
  Daniel Carvalho: Looks good to me, approved
  Gabe Black: Looks good to me, approved
  kokoro: Regressions pass




diff --git a/src/arch/arm/insts/macromem.hh b/src/arch/arm/insts/macromem.hh
index 8541131..1dd69ea 100644
--- a/src/arch/arm/insts/macromem.hh
+++ b/src/arch/arm/insts/macromem.hh
@@ -42,6 +42,7 @@
 #define __ARCH_ARM_MACROMEM_HH__

 #include "arch/arm/insts/pred_inst.hh"
+#include "arch/arm/pcstate.hh"
 #include "arch/arm/tlb.hh"

 namespace gem5
@@ -75,14 +76,15 @@

   public:
     void
-    advancePC(PCState &pcState) const override
+    advancePC(PCStateBase &pcState) const override
     {
+        auto &apc = pcState.as<PCState>();
         if (flags[IsLastMicroop]) {
-            pcState.uEnd();
+            apc.uEnd();
         } else if (flags[IsMicroop]) {
-            pcState.uAdvance();
+            apc.uAdvance();
         } else {
-            pcState.advance();
+            apc.advance();
         }
     }
 };
@@ -96,14 +98,15 @@

   public:
     void
-    advancePC(PCState &pcState) const override
+    advancePC(PCStateBase &pcState) const override
     {
+        auto &apc = pcState.as<PCState>();
         if (flags[IsLastMicroop]) {
-            pcState.uEnd();
+            apc.uEnd();
         } else if (flags[IsMicroop]) {
-            pcState.uAdvance();
+            apc.uAdvance();
         } else {
-            pcState.advance();
+            apc.advance();
         }
     }
 };
diff --git a/src/arch/arm/insts/mem.hh b/src/arch/arm/insts/mem.hh
index 7bee981..4a2c8a0 100644
--- a/src/arch/arm/insts/mem.hh
+++ b/src/arch/arm/insts/mem.hh
@@ -42,6 +42,7 @@
 #define __ARCH_ARM_MEM_HH__

 #include "arch/arm/insts/pred_inst.hh"
+#include "arch/arm/pcstate.hh"

 namespace gem5
 {
@@ -57,14 +58,15 @@
     {}

     void
-    advancePC(PCState &pcState) const override
+    advancePC(PCStateBase &pcState) const override
     {
+        auto &apc = pcState.as<PCState>();
         if (flags[IsLastMicroop]) {
-            pcState.uEnd();
+            apc.uEnd();
         } else if (flags[IsMicroop]) {
-            pcState.uAdvance();
+            apc.uAdvance();
         } else {
-            pcState.advance();
+            apc.advance();
         }
     }
 };
diff --git a/src/arch/arm/insts/mem64.hh b/src/arch/arm/insts/mem64.hh
index 7fb168c..e2dd5dd 100644
--- a/src/arch/arm/insts/mem64.hh
+++ b/src/arch/arm/insts/mem64.hh
@@ -40,6 +40,7 @@

 #include "arch/arm/insts/misc64.hh"
 #include "arch/arm/insts/static_inst.hh"
+#include "arch/arm/pcstate.hh"

 namespace gem5
 {
@@ -75,14 +76,15 @@
     {}

     void
-    advancePC(PCState &pcState) const override
+    advancePC(PCStateBase &pcState) const override
     {
+        auto &apc = pcState.as<PCState>();
         if (flags[IsLastMicroop]) {
-            pcState.uEnd();
+            apc.uEnd();
         } else if (flags[IsMicroop]) {
-            pcState.uAdvance();
+            apc.uAdvance();
         } else {
-            pcState.advance();
+            apc.advance();
         }
     }
 };
diff --git a/src/arch/arm/insts/pred_inst.hh b/src/arch/arm/insts/pred_inst.hh
index 2e35622..1f62614 100644
--- a/src/arch/arm/insts/pred_inst.hh
+++ b/src/arch/arm/insts/pred_inst.hh
@@ -42,6 +42,7 @@
 #define __ARCH_ARM_INSTS_PREDINST_HH__

 #include "arch/arm/insts/static_inst.hh"
+#include "arch/arm/pcstate.hh"
 #include "base/compiler.hh"
 #include "base/logging.hh"
 #include "base/trace.hh"
@@ -391,12 +392,13 @@
     }

     void
-    advancePC(PCState &pcState) const override
+    advancePC(PCStateBase &pcState) const override
     {
+        auto &apc = pcState.as<PCState>();
         if (flags[IsLastMicroop])
-            pcState.uEnd();
+            apc.uEnd();
         else
-            pcState.uAdvance();
+            apc.uAdvance();
     }
 };

diff --git a/src/arch/arm/insts/static_inst.hh b/src/arch/arm/insts/static_inst.hh
index 95e7f09..3979a49 100644
--- a/src/arch/arm/insts/static_inst.hh
+++ b/src/arch/arm/insts/static_inst.hh
@@ -46,6 +46,7 @@
 #include "arch/arm/faults.hh"
 #include "arch/arm/utility.hh"
 #include "arch/arm/isa.hh"
+#include "arch/arm/pcstate.hh"
 #include "arch/arm/self_debug.hh"
 #include "arch/arm/system.hh"
 #include "base/trace.hh"
@@ -197,9 +198,9 @@
                        uint64_t imm) const;

     void
-    advancePC(PCState &pcState) const override
+    advancePC(PCStateBase &pcState) const override
     {
-        pcState.advance();
+        pcState.as<PCState>().advance();
     }

     uint64_t getEMI() const override { return machInst; }
diff --git a/src/arch/arm/insts/vfp.hh b/src/arch/arm/insts/vfp.hh
index 5f5b374..76b727e 100644
--- a/src/arch/arm/insts/vfp.hh
+++ b/src/arch/arm/insts/vfp.hh
@@ -43,6 +43,7 @@
 #include <cmath>

 #include "arch/arm/insts/misc.hh"
+#include "arch/arm/pcstate.hh"
 #include "arch/arm/regs/misc.hh"

 namespace gem5
@@ -853,14 +854,15 @@
             bool flush, uint32_t rMode) const;

     void
-    advancePC(PCState &pcState) const override
+    advancePC(PCStateBase &pcState) const override
     {
+        auto &apc = pcState.as<PCState>();
         if (flags[IsLastMicroop]) {
-            pcState.uEnd();
+            apc.uEnd();
         } else if (flags[IsMicroop]) {
-            pcState.uAdvance();
+            apc.uAdvance();
         } else {
-            pcState.advance();
+            apc.advance();
         }
     }

diff --git a/src/arch/mips/isa/base.isa b/src/arch/mips/isa/base.isa
index bfc46c2..b02a8c4 100644
--- a/src/arch/mips/isa/base.isa
+++ b/src/arch/mips/isa/base.isa
@@ -59,9 +59,9 @@
         ExtMachInst machInst;

         void
-        advancePC(MipsISA::PCState &pc) const override
+        advancePC(PCStateBase &pc) const override
         {
-            pc.advance();
+            pc.as<PCState>().advance();
         }

         PCState
diff --git a/src/arch/power/insts/static_inst.hh b/src/arch/power/insts/static_inst.hh
index 54ef589..bbe93fe 100644
--- a/src/arch/power/insts/static_inst.hh
+++ b/src/arch/power/insts/static_inst.hh
@@ -29,6 +29,7 @@
 #ifndef __ARCH_POWER_INSTS_STATICINST_HH__
 #define __ARCH_POWER_INSTS_STATICINST_HH__

+#include "arch/power/pcstate.hh"
 #include "arch/power/types.hh"
 #include "base/trace.hh"
 #include "cpu/static_inst.hh"
@@ -68,9 +69,9 @@
             Addr pc, const loader::SymbolTable *symtab) const override;

     void
-    advancePC(PowerISA::PCState &pcState) const override
+    advancePC(PCStateBase &pc_state) const override
     {
-        pcState.advance();
+        pc_state.as<PCState>().advance();
     }

     PCState
diff --git a/src/arch/riscv/insts/static_inst.cc b/src/arch/riscv/insts/static_inst.cc
index de40f67..ee8ab43 100644
--- a/src/arch/riscv/insts/static_inst.cc
+++ b/src/arch/riscv/insts/static_inst.cc
@@ -29,6 +29,7 @@

 #include "arch/riscv/insts/static_inst.hh"

+#include "arch/riscv/pcstate.hh"
 #include "arch/riscv/types.hh"
 #include "cpu/static_inst.hh"

@@ -39,12 +40,13 @@
 {

 void
-RiscvMicroInst::advancePC(PCState &pcState) const
+RiscvMicroInst::advancePC(PCStateBase &pcState) const
 {
+    auto &rpc = pcState.as<PCState>();
     if (flags[IsLastMicroop]) {
-        pcState.uEnd();
+        rpc.uEnd();
     } else {
-        pcState.uAdvance();
+        rpc.uAdvance();
     }
 }

diff --git a/src/arch/riscv/insts/static_inst.hh b/src/arch/riscv/insts/static_inst.hh
index 867f9a4..1e2ec28 100644
--- a/src/arch/riscv/insts/static_inst.hh
+++ b/src/arch/riscv/insts/static_inst.hh
@@ -32,6 +32,7 @@

 #include <string>

+#include "arch/riscv/pcstate.hh"
 #include "arch/riscv/types.hh"
 #include "cpu/exec_context.hh"
 #include "cpu/static_inst.hh"
@@ -57,7 +58,11 @@
   public:
     ExtMachInst machInst;

-    void advancePC(PCState &pc) const override { pc.advance(); }
+    void
+    advancePC(PCStateBase &pc) const override
+    {
+        pc.as<PCState>().advance();
+    }

     PCState
     buildRetPC(const PCState &curPC, const PCState &callPC) const override
@@ -131,7 +136,7 @@
         flags[IsMicroop] = true;
     }

-    void advancePC(PCState &pcState) const override;
+    void advancePC(PCStateBase &pcState) const override;
 };

 } // namespace RiscvISA
diff --git a/src/arch/sparc/insts/micro.hh b/src/arch/sparc/insts/micro.hh
index 2d89189..b07e8c2 100644
--- a/src/arch/sparc/insts/micro.hh
+++ b/src/arch/sparc/insts/micro.hh
@@ -101,12 +101,13 @@
     }

     void
-    advancePC(SparcISA::PCState &pcState) const override
+    advancePC(PCStateBase &pc_state) const override
     {
+        auto &spc = pc_state.as<PCState>();
         if (flags[IsLastMicroop])
-            pcState.uEnd();
+            spc.uEnd();
         else
-            pcState.uAdvance();
+            spc.uAdvance();
     }
 };

diff --git a/src/arch/sparc/insts/static_inst.cc b/src/arch/sparc/insts/static_inst.cc
index 2b35783..d6da2b0 100644
--- a/src/arch/sparc/insts/static_inst.cc
+++ b/src/arch/sparc/insts/static_inst.cc
@@ -29,6 +29,7 @@

 #include "arch/sparc/insts/static_inst.hh"

+#include "arch/sparc/pcstate.hh"
 #include "arch/sparc/regs/int.hh"
 #include "arch/sparc/regs/misc.hh"
 #include "base/bitunion.hh"
@@ -79,9 +80,9 @@
 }

 void
-SparcStaticInst::advancePC(SparcISA::PCState &pcState) const
+SparcStaticInst::advancePC(PCStateBase &pcState) const
 {
-    pcState.advance();
+    pcState.as<PCState>().advance();
 }

 void
diff --git a/src/arch/sparc/insts/static_inst.hh b/src/arch/sparc/insts/static_inst.hh
index 5b67b80..b2dfd5e 100644
--- a/src/arch/sparc/insts/static_inst.hh
+++ b/src/arch/sparc/insts/static_inst.hh
@@ -32,6 +32,7 @@

 #include <cstdint>

+#include "arch/sparc/pcstate.hh"
 #include "arch/sparc/types.hh"
 #include "base/trace.hh"
 #include "cpu/exec_context.hh"
@@ -110,7 +111,7 @@
     void printRegArray(std::ostream &os,
         const RegId *indexArray, int num) const;

-    void advancePC(PCState &pcState) const override;
+    void advancePC(PCStateBase &pcState) const override;

     static bool passesFpCondition(uint32_t fcc, uint32_t condition);
     static bool passesCondition(uint32_t codes, uint32_t condition);
diff --git a/src/arch/x86/insts/microop.hh b/src/arch/x86/insts/microop.hh
index c59c5b9..350294f 100644
--- a/src/arch/x86/insts/microop.hh
+++ b/src/arch/x86/insts/microop.hh
@@ -39,6 +39,7 @@
 #define __ARCH_X86_INSTS_MICROOP_HH__

 #include "arch/x86/insts/static_inst.hh"
+#include "arch/x86/pcstate.hh"
 #include "base/compiler.hh"

 namespace gem5
@@ -132,12 +133,13 @@
     bool checkCondition(uint64_t flags, int condition) const;

     void
-    advancePC(PCState &pcState) const override
+    advancePC(PCStateBase &pcState) const override
     {
+        auto &xpc = pcState.as<PCState>();
         if (flags[IsLastMicroop])
-            pcState.uEnd();
+            xpc.uEnd();
         else
-            pcState.uAdvance();
+            xpc.uAdvance();
     }

     PCState branchTarget(const PCState &branchPC) const override;
diff --git a/src/arch/x86/insts/static_inst.hh b/src/arch/x86/insts/static_inst.hh
index 3b2b9f6..1654e81 100644
--- a/src/arch/x86/insts/static_inst.hh
+++ b/src/arch/x86/insts/static_inst.hh
@@ -38,6 +38,7 @@
 #ifndef __ARCH_X86_INSTS_STATICINST_HH__
 #define __ARCH_X86_INSTS_STATICINST_HH__

+#include "arch/x86/pcstate.hh"
 #include "arch/x86/types.hh"
 #include "base/trace.hh"
 #include "cpu/static_inst.hh"
@@ -198,9 +199,9 @@
     }

     void
-    advancePC(PCState &pcState) const override
+    advancePC(PCStateBase &pcState) const override
     {
-        pcState.advance();
+        pcState.as<PCState>().advance();
     }

     PCState
diff --git a/src/cpu/nop_static_inst.cc b/src/cpu/nop_static_inst.cc
index 8baf948..b9a69dc 100644
--- a/src/cpu/nop_static_inst.cc
+++ b/src/cpu/nop_static_inst.cc
@@ -28,6 +28,7 @@

 #include "cpu/nop_static_inst.hh"

+#include "arch/pcstate.hh"
 #include "cpu/static_inst.hh"

 namespace gem5
@@ -48,9 +49,9 @@
     }

     void
-    advancePC(TheISA::PCState &pcState) const override
+    advancePC(PCStateBase &pcState) const override
     {
-        pcState.advance();
+        pcState.as<TheISA::PCState>().advance();
     }

     std::string
diff --git a/src/cpu/static_inst.hh b/src/cpu/static_inst.hh
index 2039644..d8c09ad 100644
--- a/src/cpu/static_inst.hh
+++ b/src/cpu/static_inst.hh
@@ -320,7 +320,7 @@
         panic("completeAcc not defined!");
     }

-    virtual void advancePC(TheISA::PCState &pc_state) const = 0;
+    virtual void advancePC(PCStateBase &pc_state) const = 0;

     virtual TheISA::PCState
     buildRetPC(const TheISA::PCState &cur_pc,

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/52043
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: I774c67dc648a85556230f601e087211b3d5630a9
Gerrit-Change-Number: 52043
Gerrit-PatchSet: 14
Gerrit-Owner: Gabe Black <[email protected]>
Gerrit-Reviewer: Daniel Carvalho <[email protected]>
Gerrit-Reviewer: Gabe Black <[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

Reply via email to