Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/42683 )

Change subject: cpu: Eliminate the isZeroReg() helper in RegId.
......................................................................

cpu: Eliminate the isZeroReg() helper in RegId.

The isZeroReg() helper checked if the register was both an integer
register, and if it equaled TheISA::ZeroReg. This bakes in both the
assumption that any zero registers are integer (and that integer
registers are a thing), and also internalizes the plumbing which selects
what index is the zero register.

This change eliminates the isZeroReg helper and moves the logic inside
it into where it was called. In most cases, it was actually not
necessary to check if the register was integer since that was already
implied by context. This also brings the TheISA::ZeroReg constant out,
where it can be replaced by values plumbed in more generally than a
fixed, ISA specific constant.

Change-Id: I651762b6eb01fea83ec0b0076e8be9bf24b5b0da
---
M src/cpu/minor/dyn_inst.cc
M src/cpu/minor/scoreboard.cc
M src/cpu/o3/probe/elastic_trace.cc
M src/cpu/o3/regfile.hh
M src/cpu/o3/rename_map.cc
M src/cpu/o3/scoreboard.hh
M src/cpu/reg_class.hh
7 files changed, 45 insertions(+), 57 deletions(-)



diff --git a/src/cpu/minor/dyn_inst.cc b/src/cpu/minor/dyn_inst.cc
index 8723488..510ec10 100644
--- a/src/cpu/minor/dyn_inst.cc
+++ b/src/cpu/minor/dyn_inst.cc
@@ -163,7 +163,7 @@
               static_cast<unsigned int>(reg.elemIndex()) << ']';
         break;
       case IntRegClass:
-        if (reg.isZeroReg()) {
+        if (reg.index() == TheISA::ZeroReg) {
             os << 'z';
         } else {
             os << 'r' << static_cast<unsigned int>(reg.index());
diff --git a/src/cpu/minor/scoreboard.cc b/src/cpu/minor/scoreboard.cc
index 0d943f1..5f8df87 100644
--- a/src/cpu/minor/scoreboard.cc
+++ b/src/cpu/minor/scoreboard.cc
@@ -50,41 +50,39 @@
 {
     bool ret = false;

-    if (reg.isZeroReg()) {
-        /* Don't bother with the zero register */
-        ret = false;
-    } else {
-        switch (reg.classValue())
-        {
-          case IntRegClass:
+    switch (reg.classValue()) {
+      case IntRegClass:
+        if (reg.index() == TheISA::ZeroReg) {
+            /* Don't bother with the zero register */
+            ret = false;
+        } else {
             scoreboard_index = reg.index();
             ret = true;
-            break;
-          case FloatRegClass:
-            scoreboard_index = floatRegOffset + reg.index();
-            ret = true;
-            break;
-          case VecRegClass:
-          case VecElemClass:
-            scoreboard_index = vecRegOffset + reg.index();
-            ret = true;
-            break;
-          case VecPredRegClass:
-            scoreboard_index = vecPredRegOffset + reg.index();
-            ret = true;
-            break;
-          case CCRegClass:
-            scoreboard_index = ccRegOffset + reg.index();
-            ret = true;
-            break;
-          case MiscRegClass:
-              /* Don't bother with Misc registers */
-            ret = false;
-            break;
-          default:
-            panic("Unknown register class: %d",
-                    static_cast<int>(reg.classValue()));
         }
+        break;
+      case FloatRegClass:
+        scoreboard_index = floatRegOffset + reg.index();
+        ret = true;
+        break;
+      case VecRegClass:
+      case VecElemClass:
+        scoreboard_index = vecRegOffset + reg.index();
+        ret = true;
+        break;
+      case VecPredRegClass:
+        scoreboard_index = vecPredRegOffset + reg.index();
+        ret = true;
+        break;
+      case CCRegClass:
+        scoreboard_index = ccRegOffset + reg.index();
+        ret = true;
+        break;
+      case MiscRegClass:
+          /* Don't bother with Misc registers */
+        ret = false;
+        break;
+      default:
+        panic("Unknown register class: %d", reg.classValue());
     }

     return ret;
diff --git a/src/cpu/o3/probe/elastic_trace.cc b/src/cpu/o3/probe/elastic_trace.cc
index ddea041..b48504a 100644
--- a/src/cpu/o3/probe/elastic_trace.cc
+++ b/src/cpu/o3/probe/elastic_trace.cc
@@ -241,7 +241,7 @@

         const RegId& src_reg = dyn_inst->srcRegIdx(src_idx);
         if (!src_reg.isMiscReg() &&
-            !src_reg.isZeroReg()) {
+ !(src_reg.isIntReg() && src_reg.index() == TheISA::ZeroReg)) {
             // Get the physical register index of the i'th source register.
PhysRegIdPtr phys_src_reg = dyn_inst->regs.renamedSrcIdx(src_idx);
             DPRINTFR(ElasticTrace, "[sn:%lli] Check map for src reg"
@@ -273,7 +273,8 @@
         // CC register and not a Misc register.
         const RegId& dest_reg = dyn_inst->destRegIdx(dest_idx);
         if (!dest_reg.isMiscReg() &&
-            !dest_reg.isZeroReg()) {
+                !(dest_reg.isIntReg() &&
+                    dest_reg.index() == TheISA::ZeroReg)) {
             // Get the physical register index of the i'th destination
             // register.
             PhysRegIdPtr phys_dest_reg =
diff --git a/src/cpu/o3/regfile.hh b/src/cpu/o3/regfile.hh
index d30f577..adfa326 100644
--- a/src/cpu/o3/regfile.hh
+++ b/src/cpu/o3/regfile.hh
@@ -274,7 +274,7 @@
         DPRINTF(IEW, "RegFile: Setting int register %i to %#x\n",
                 phys_reg->index(), val);

-        if (!phys_reg->isZeroReg())
+        if (phys_reg->index() != TheISA::ZeroReg)
             intRegFile[phys_reg->index()] = val;
     }

@@ -286,8 +286,7 @@
         DPRINTF(IEW, "RegFile: Setting float register %i to %#x\n",
                 phys_reg->index(), (uint64_t)val);

-        if (!phys_reg->isZeroReg())
-            floatRegFile[phys_reg->index()] = val;
+        floatRegFile[phys_reg->index()] = val;
     }

     /** Sets a vector register to the given value. */
diff --git a/src/cpu/o3/rename_map.cc b/src/cpu/o3/rename_map.cc
index 5579929..e26ef9b 100644
--- a/src/cpu/o3/rename_map.cc
+++ b/src/cpu/o3/rename_map.cc
@@ -76,7 +76,7 @@
     PhysRegIdPtr prev_reg = map[arch_reg.flatIndex()];

     if (arch_reg == zeroReg) {
-        assert(prev_reg->isZeroReg());
+        assert(prev_reg->index() == TheISA::ZeroReg);
         renamed_reg = prev_reg;
     } else if (prev_reg->getNumPinnedWrites() > 0) {
         // Do not rename if the register is pinned
diff --git a/src/cpu/o3/scoreboard.hh b/src/cpu/o3/scoreboard.hh
index 0a37b40..1873116 100644
--- a/src/cpu/o3/scoreboard.hh
+++ b/src/cpu/o3/scoreboard.hh
@@ -72,7 +72,8 @@
     std::string name() const { return _name; };

     /** Checks if the register is ready. */
-    bool getReg(PhysRegIdPtr phys_reg) const
+    bool
+    getReg(PhysRegIdPtr phys_reg) const
     {
         assert(phys_reg->flatIndex() < numPhysRegs);

@@ -83,14 +84,15 @@

         bool ready = regScoreBoard[phys_reg->flatIndex()];

-        if (phys_reg->isZeroReg())
+ if (phys_reg->isIntPhysReg() && phys_reg->index() == TheISA::ZeroReg)
             assert(ready);

         return ready;
     }

     /** Sets the register as ready. */
-    void setReg(PhysRegIdPtr phys_reg)
+    void
+    setReg(PhysRegIdPtr phys_reg)
     {
         assert(phys_reg->flatIndex() < numPhysRegs);

@@ -107,7 +109,8 @@
     }

     /** Sets the register as not ready. */
-    void unsetReg(PhysRegIdPtr phys_reg)
+    void
+    unsetReg(PhysRegIdPtr phys_reg)
     {
         assert(phys_reg->flatIndex() < numPhysRegs);

@@ -118,7 +121,7 @@
         }

         // zero reg should never be marked unready
-        if (phys_reg->isZeroReg())
+ if (phys_reg->isIntPhysReg() && phys_reg->index() == TheISA::ZeroReg)
             return;

         regScoreBoard[phys_reg->flatIndex()] = false;
diff --git a/src/cpu/reg_class.hh b/src/cpu/reg_class.hh
index 3496874..f1d9369 100644
--- a/src/cpu/reg_class.hh
+++ b/src/cpu/reg_class.hh
@@ -144,18 +144,6 @@
         return regClass != MiscRegClass;
     }

-    /**
-     * Check if this is the zero register.
-     * Returns true if this register is a zero register (needs to have a
-     * constant zero value throughout the execution).
-     */
-
-    inline bool
-    isZeroReg() const
-    {
-        return regClass == IntRegClass && regIdx == TheISA::ZeroReg;
-    }
-
     /** @return true if it is an integer physical register. */
     bool isIntReg() const { return regClass == IntRegClass; }

@@ -261,7 +249,6 @@
     /** @{ */
     using RegId::index;
     using RegId::classValue;
-    using RegId::isZeroReg;
     using RegId::className;
     using RegId::elemIndex;
      /** @} */

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/42683
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: I651762b6eb01fea83ec0b0076e8be9bf24b5b0da
Gerrit-Change-Number: 42683
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black <[email protected]>
Gerrit-MessageType: newchange
_______________________________________________
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