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

Change subject: arch-x86: Eliminate the DependenceTags in registers.hh.
......................................................................

arch-x86: Eliminate the DependenceTags in registers.hh.

These were a weird holdover from when register indices were all squished
into a single scalar value, offset based on the register type. When that
change happened, the person who made it misunderstood what the
InstRegIndex type was for, and decided to build RegId into it for some
reason. The only purpose of InstRegIndex is to make sure everybody
agrees that the value being passed around is going to be used as a
register index and not a literal value, or vice versa. There is no type
associated with it as far as floating point, integer, or misc registers.
That gets applied at a different step, and actually can't be part of
InstRegIndex since the same base class may need to hold register indices
that are going to be treated as integer or floating point depending on
the subclass using them.

Also, since the values of the various constants in the DepdenceTags enum
where never actually added into register indices in the first place, the
code in the InstRegIndex constructor would never do anything. All
registers would be arbitrarily sorted into Int, FP, etc, and then when
they actually went to be used the category would be thrown away.

Change-Id: I8c4e8d6e9cdb53e298c00ad2f56e8c7304e51e96
---
M src/arch/x86/insts/microfpop.hh
M src/arch/x86/insts/microldstop.hh
M src/arch/x86/insts/micromediaop.hh
M src/arch/x86/insts/microregop.hh
M src/arch/x86/insts/static_inst.cc
M src/arch/x86/insts/static_inst.hh
M src/arch/x86/isa/microops/limmop.isa
M src/arch/x86/isa/specialize.isa
M src/arch/x86/registers.hh
9 files changed, 41 insertions(+), 64 deletions(-)



diff --git a/src/arch/x86/insts/microfpop.hh b/src/arch/x86/insts/microfpop.hh
index e9d32da..13126b1 100644
--- a/src/arch/x86/insts/microfpop.hh
+++ b/src/arch/x86/insts/microfpop.hh
@@ -64,7 +64,7 @@
             OpClass __opClass) :
         X86MicroopBase(_machInst, mnem, _instMnem, setFlags,
                 __opClass),
-        src1(_src1.index()), src2(_src2.index()), dest(_dest.index()),
+        src1(_src1.index), src2(_src2.index), dest(_dest.index),
         dataSize(_dataSize), spm(_spm)
     {}

diff --git a/src/arch/x86/insts/microldstop.hh b/src/arch/x86/insts/microldstop.hh
index 2611095..955df58 100644
--- a/src/arch/x86/insts/microldstop.hh
+++ b/src/arch/x86/insts/microldstop.hh
@@ -74,12 +74,12 @@
             Request::FlagsType _memFlags,
             OpClass __opClass) :
     X86MicroopBase(_machInst, mnem, _instMnem, setFlags, __opClass),
-            scale(_scale), index(_index.index()), base(_base.index()),
-            disp(_disp), segment(_segment.index()),
+            scale(_scale), index(_index.index), base(_base.index),
+            disp(_disp), segment(_segment.index),
             dataSize(_dataSize), addressSize(_addressSize),
-            memFlags(_memFlags | _segment.index())
+            memFlags(_memFlags | _segment.index)
     {
-        assert(_segment.index() < NUM_SEGMENTREGS);
+        assert(_segment.index < NUM_SEGMENTREGS);
         foldOBit = (dataSize == 1 && !_machInst.rex.present) ? 1 << 6 : 0;
foldABit = (addressSize == 1 && !_machInst.rex.present) ? 1 << 6 : 0;
     }
@@ -107,7 +107,7 @@
             _scale, _index, _base, _disp, _segment,
             _dataSize, _addressSize, _memFlags,
             __opClass),
-            data(_data.index())
+            data(_data.index)
     {
     }

@@ -140,8 +140,8 @@
             _scale, _index, _base, _disp, _segment,
             _dataSize, _addressSize, _memFlags,
             __opClass),
-            dataLow(_dataLow.index()),
-            dataHi(_dataHi.index())
+            dataLow(_dataLow.index),
+            dataHi(_dataHi.index)
     {
     }

diff --git a/src/arch/x86/insts/micromediaop.hh b/src/arch/x86/insts/micromediaop.hh
index 04e3da8..94818da 100644
--- a/src/arch/x86/insts/micromediaop.hh
+++ b/src/arch/x86/insts/micromediaop.hh
@@ -59,7 +59,7 @@
             OpClass __opClass) :
         X86MicroopBase(_machInst, mnem, _instMnem, setFlags,
                 __opClass),
-        src1(_src1.index()), dest(_dest.index()),
+        src1(_src1.index), dest(_dest.index),
         srcSize(_srcSize), destSize(_destSize), ext(_ext)
     {}

@@ -102,7 +102,7 @@
         MediaOpBase(_machInst, mnem, _instMnem, setFlags,
                 _src1, _dest, _srcSize, _destSize, _ext,
                 __opClass),
-        src2(_src2.index())
+        src2(_src2.index)
     {}

     std::string generateDisassembly(Addr pc,
diff --git a/src/arch/x86/insts/microregop.hh b/src/arch/x86/insts/microregop.hh
index 1c26111..3129518 100644
--- a/src/arch/x86/insts/microregop.hh
+++ b/src/arch/x86/insts/microregop.hh
@@ -63,7 +63,7 @@
             OpClass __opClass) :
         X86MicroopBase(_machInst, mnem, _instMnem, setFlags,
                 __opClass),
-        src1(_src1.index()), dest(_dest.index()),
+        src1(_src1.index), dest(_dest.index),
         dataSize(_dataSize), ext(_ext)
     {
         foldOBit = (dataSize == 1 && !_machInst.rex.present) ? 1 << 6 : 0;
@@ -89,7 +89,7 @@
         RegOpBase(_machInst, mnem, _instMnem, setFlags,
                 _src1, _dest, _dataSize, _ext,
                 __opClass),
-        src2(_src2.index())
+        src2(_src2.index)
     {
     }

diff --git a/src/arch/x86/insts/static_inst.cc b/src/arch/x86/insts/static_inst.cc
index c23d014..e49f5ec 100644
--- a/src/arch/x86/insts/static_inst.cc
+++ b/src/arch/x86/insts/static_inst.cc
@@ -243,13 +243,13 @@
         if (scale != 0 && index != ZeroReg) {
             if (scale != 1)
                 ccprintf(os, "%d*", scale);
-            printReg(os, InstRegIndex(index), addressSize);
+            printReg(os, RegId(IntRegClass, index), addressSize);
             someAddr = true;
         }
         if (base != ZeroReg) {
             if (someAddr)
                 os << " + ";
-            printReg(os, InstRegIndex(base), addressSize);
+            printReg(os, RegId(IntRegClass, base), addressSize);
             someAddr = true;
         }
     }
diff --git a/src/arch/x86/insts/static_inst.hh b/src/arch/x86/insts/static_inst.hh
index 92c5c03..27a27b8 100644
--- a/src/arch/x86/insts/static_inst.hh
+++ b/src/arch/x86/insts/static_inst.hh
@@ -51,29 +51,10 @@
  * wrapper struct for these lets take advantage of the compiler's type
  * checking.
  */
-struct InstRegIndex : public RegId
+struct InstRegIndex
 {
-    explicit InstRegIndex(RegIndex _idx) :
-       RegId(computeRegClass(_idx), _idx) {}
-
-  private:
-    // TODO: As X86 register index definition is highly built on the
-    //       unified space concept, it is easier for the moment to rely on
-    //       an helper function to compute the RegClass. It would be nice
-    //       to fix those definition and get rid of this.
-    RegClass
-    computeRegClass(RegIndex _idx)
-    {
-        if (_idx < FP_Reg_Base) {
-            return IntRegClass;
-        } else if (_idx < CC_Reg_Base) {
-            return FloatRegClass;
-        } else if (_idx < Misc_Reg_Base) {
-            return CCRegClass;
-        } else {
-            return MiscRegClass;
-        }
-    }
+    RegIndex index;
+    explicit InstRegIndex(RegIndex idx) : index(idx) {}
 };

 /**
diff --git a/src/arch/x86/isa/microops/limmop.isa b/src/arch/x86/isa/microops/limmop.isa
index 51310b4..4f57f31 100644
--- a/src/arch/x86/isa/microops/limmop.isa
+++ b/src/arch/x86/isa/microops/limmop.isa
@@ -97,7 +97,7 @@
             InstRegIndex _dest, uint64_t _imm, uint8_t _dataSize) :
         %(base_class)s(machInst, "%(mnemonic)s", instMnem,
                 setFlags, %(op_class)s),
-                dest(_dest.index()), imm(_imm), dataSize(_dataSize)
+                dest(_dest.index), imm(_imm), dataSize(_dataSize)
     {
         %(set_reg_idx_arr)s;
         foldOBit = (dataSize == 1 && !machInst.rex.present) ? 1 << 6 : 0;
diff --git a/src/arch/x86/isa/specialize.isa b/src/arch/x86/isa/specialize.isa
index 946732f..1d21d87 100644
--- a/src/arch/x86/isa/specialize.isa
+++ b/src/arch/x86/isa/specialize.isa
@@ -142,16 +142,17 @@
                     regString = "INTREG_R%s" % opType.reg
                 env.addReg(regString)
                 env.addToDisassembly(
-                    "printReg(out, InstRegIndex(%s), regSize);\n" %
- regString)
+                    "printReg(out, RegId(IntRegClass, %s), regSize);\n" %
+                    regString)

                 Name += "_R"

             elif opType.tag == "B":
- # This refers to registers whose index is encoded as part of the opcode + # This refers to registers whose index is encoded as part of
+                # the opcode.
                 env.addToDisassembly(
-                        "printReg(out, InstRegIndex(%s), regSize);\n" %
- InstRegIndex) + "printReg(out, RegId(IntRegClass, %s), regSize);\n" %
+                        InstRegIndex)

                 Name += "_R"

@@ -187,36 +188,40 @@
             elif opType.tag in ("G", "P", "T", "V"):
# Use the "reg" field of the ModRM byte to select the register
                 env.addReg(ModRMRegIndex)
-                env.addToDisassembly(
-                        "printReg(out, InstRegIndex(%s), regSize);\n" %
- ModRMRegIndex)

                 if opType.tag == "P":
-
+                    regFormat = 'ccprintf(out, "MMX%%d", %s);\n'
                     Name += "_MMX"
                 elif opType.tag == "V":
+                    regFormat = 'ccprintf(out, "XMM%%d", %s);\n'
                     Name += "_XMM"
                 else:
+                    regFormat = \
+                        "printReg(out, RegId(IntRegClass, %s), regSize);\n"
                     Name += "_R"
+                env.addToDisassembly(regFormat % ModRMRegIndex)
             elif opType.tag in ("E", "Q", "W"):
                 # This might refer to memory or to a register. We need to
                 # divide it up farther.
                 regEnv = copy.copy(env)
                 regEnv.addReg(ModRMRMIndex)
-                regEnv.addToDisassembly(
-                        "printReg(out, InstRegIndex(%s), regSize);\n" %
- ModRMRMIndex)

# This refers to memory. The macroop constructor should set up

                 # modrm addressing.
                 memEnv = copy.copy(env)
                 memEnv.doModRM = True
-                regSuffix = "_R"
                 if opType.tag == "Q":
+                    regFormat = 'ccprintf(out, "MMX%%d", %s);\n'
                     regSuffix = "_MMX"
                 elif opType.tag == "W":
+                    regFormat = 'ccprintf(out, "XMM%%d", %s);\n'
                     regSuffix = "_XMM"
+                else:
+                    regFormat = \
+                        "printReg(out, RegId(IntRegClass, %s), regSize);\n"
+                    regSuffix = "_R"
+                env.addToDisassembly(regFormat % ModRMRegIndex)
                 return doSplitDecode("MODRM_MOD",
                     {"3" : (specializeInst, Name + regSuffix,
                             copy.copy(opTypes), regEnv)},
@@ -233,17 +238,18 @@
             elif opType.tag in ("PR", "R", "VR"):
                 # Non register modrm settings should cause an error
                 env.addReg(ModRMRMIndex)
-                env.addToDisassembly(
-                        "printReg(out, InstRegIndex(%s), regSize);\n" %
- ModRMRMIndex)

                 if opType.tag == "PR":
-
+                    regFormat = 'ccprintf(out, "MMX%%d", %s);\n'
                     Name += "_MMX"
                 elif opType.tag == "VR":
+                    regFormat = 'ccprintf(out, "XMM%%d", %s);\n'
                     Name += "_XMM"
                 else:
+                    regFormat = \
+                        "printReg(out, RegId(IntRegClass, %s), regSize);\n"
                     Name += "_R"
+                env.addToDisassembly(regFormat % ModRMRegIndex)
             elif opType.tag in ("X", "Y"):
# This type of memory addressing is for string instructions.
                 # They'll use the right index and segment internally.
diff --git a/src/arch/x86/registers.hh b/src/arch/x86/registers.hh
index e89f4e4..fcdfe8d 100644
--- a/src/arch/x86/registers.hh
+++ b/src/arch/x86/registers.hh
@@ -63,16 +63,6 @@
 const int NumFloatRegs =
     NumMMXRegs + 2 * NumXMMRegs + NumMicroFpRegs + 8;

-// These enumerate all the registers for dependence tracking.
-enum DependenceTags {
-    // FP_Reg_Base must be large enough to be bigger than any integer
-    // register index which has the IntFoldBit (1 << 6) set.  To be safe
-    // we just start at (1 << 7) == 128.
-    FP_Reg_Base = 128,
-    CC_Reg_Base = FP_Reg_Base + NumFloatRegs,
-    Misc_Reg_Base = CC_Reg_Base + NumCCRegs
-};
-
 const int NumVecRegs = 1;  // Not applicable to x86
                            // (1 to prevent warnings)
 const int NumVecPredRegs = 1;  // Not applicable to x86

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/40339
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: I8c4e8d6e9cdb53e298c00ad2f56e8c7304e51e96
Gerrit-Change-Number: 40339
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