Gabe Black has submitted this change. (
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
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/40339
Reviewed-by: Gabe Black <gabe.bl...@gmail.com>
Maintainer: Gabe Black <gabe.bl...@gmail.com>
Tested-by: kokoro <noreply+kok...@google.com>
---
M src/arch/x86/insts/microldstop.hh
M src/arch/x86/insts/microop_args.hh
M src/arch/x86/insts/static_inst.cc
M src/arch/x86/insts/static_inst.hh
M src/arch/x86/isa/specialize.isa
M src/arch/x86/vecregs.hh
6 files changed, 42 insertions(+), 67 deletions(-)
Approvals:
Gabe Black: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/arch/x86/insts/microldstop.hh
b/src/arch/x86/insts/microldstop.hh
index 1272cb0..8c7279d 100644
--- a/src/arch/x86/insts/microldstop.hh
+++ b/src/arch/x86/insts/microldstop.hh
@@ -90,7 +90,7 @@
InstOperands<MemOp, FoldedDataOp, AddrOp>(
mach_inst, mnem, inst_mnem, set_flags, op_class,
_data, { _scale, _index, _base, _disp, _segment },
- data_size, address_size, mem_flags | _segment.index())
+ data_size, address_size, mem_flags | _segment.index)
{}
};
@@ -109,7 +109,7 @@
InstOperands<MemOp, FloatDataOp, AddrOp>(
mach_inst, mnem, inst_mnem, set_flags, op_class,
_data, { _scale, _index, _base, _disp, _segment },
- data_size, address_size, mem_flags | _segment.index())
+ data_size, address_size, mem_flags | _segment.index)
{}
};
@@ -127,7 +127,7 @@
InstOperands<MemOp, AddrOp>(
mach_inst, mnem, inst_mnem, set_flags, op_class,
{ _scale, _index, _base, _disp, _segment },
- data_size, address_size, mem_flags | _segment.index())
+ data_size, address_size, mem_flags | _segment.index)
{}
};
@@ -149,7 +149,7 @@
InstOperands<MemOp, FoldedDataLowOp, FoldedDataHiOp, AddrOp>(
mach_inst, mnem, inst_mnem, set_flags, op_class,
data_low, data_hi, { _scale, _index, _base, _disp, _segment },
- data_size, address_size, mem_flags | _segment.index())
+ data_size, address_size, mem_flags | _segment.index)
{}
};
diff --git a/src/arch/x86/insts/microop_args.hh
b/src/arch/x86/insts/microop_args.hh
index 6fa09f5..85f60d1 100644
--- a/src/arch/x86/insts/microop_args.hh
+++ b/src/arch/x86/insts/microop_args.hh
@@ -108,7 +108,7 @@
{
template <class InstType>
IntOp(InstType *inst, typename Base::ArgType idx) :
- Base(idx.index(), inst->dataSize)
+ Base(idx.index, inst->dataSize)
{}
void
@@ -124,7 +124,7 @@
{
template <class InstType>
FoldedOp(InstType *inst, typename Base::ArgType idx) :
- Base(INTREG_FOLDED(idx.index(), inst->foldOBit), inst->dataSize)
+ Base(INTREG_FOLDED(idx.index, inst->foldOBit), inst->dataSize)
{}
void
@@ -139,7 +139,7 @@
struct CrOp : public Base
{
template <class InstType>
- CrOp(InstType *inst, typename Base::ArgType idx) : Base(idx.index(),
0) {}
+ CrOp(InstType *inst, typename Base::ArgType idx) : Base(idx.index, 0)
{}
void
print(std::ostream &os) const
@@ -152,7 +152,7 @@
struct DbgOp : public Base
{
template <class InstType>
- DbgOp(InstType *inst, typename Base::ArgType idx) : Base(idx.index(),
0) {}
+ DbgOp(InstType *inst, typename Base::ArgType idx) : Base(idx.index, 0)
{}
void
print(std::ostream &os) const
@@ -166,7 +166,7 @@
struct SegOp : public Base
{
template <class InstType>
- SegOp(InstType *inst, typename Base::ArgType idx) : Base(idx.index(),
0) {}
+ SegOp(InstType *inst, typename Base::ArgType idx) : Base(idx.index, 0)
{}
void
print(std::ostream &os) const
@@ -180,7 +180,7 @@
{
template <class InstType>
MiscOp(InstType *inst, typename Base::ArgType idx) :
- Base(idx.index(), inst->dataSize)
+ Base(idx.index, inst->dataSize)
{}
void
@@ -196,7 +196,7 @@
{
template <class InstType>
FloatOp(InstType *inst, typename Base::ArgType idx) :
- Base(idx.index(), inst->dataSize)
+ Base(idx.index, inst->dataSize)
{}
void
@@ -316,9 +316,9 @@
template <class InstType>
AddrOp(InstType *inst, const ArgType &args) : scale(args.scale),
- index(INTREG_FOLDED(args.index.index(), inst->foldABit)),
- base(INTREG_FOLDED(args.base.index(), inst->foldABit)),
- disp(args.disp), segment(args.segment.index()),
+ index(INTREG_FOLDED(args.index.index, inst->foldABit)),
+ base(INTREG_FOLDED(args.base.index, inst->foldABit)),
+ disp(args.disp), segment(args.segment.index),
size(inst->addressSize)
{
assert(segment < NUM_SEGMENTREGS);
diff --git a/src/arch/x86/insts/static_inst.cc
b/src/arch/x86/insts/static_inst.cc
index 71aa6e5..194342b 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 != NUM_INTREGS) {
if (scale != 1)
ccprintf(os, "%d*", scale);
- printReg(os, InstRegIndex(index), addressSize);
+ printReg(os, RegId(IntRegClass, index), addressSize);
someAddr = true;
}
if (base != NUM_INTREGS) {
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 507de19..9fadca1 100644
--- a/src/arch/x86/insts/static_inst.hh
+++ b/src/arch/x86/insts/static_inst.hh
@@ -50,29 +50,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/specialize.isa
b/src/arch/x86/isa/specialize.isa
index d6321f6..839b66d 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/vecregs.hh b/src/arch/x86/vecregs.hh
index f8c94ce..464200f 100644
--- a/src/arch/x86/vecregs.hh
+++ b/src/arch/x86/vecregs.hh
@@ -49,18 +49,6 @@
namespace X86ISA
{
-// 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 + NUM_CCREGS,
- Max_Reg_Index = Misc_Reg_Base + NUM_MISCREGS
-};
-
// Not applicable to x86
using VecElem = ::DummyVecElem;
using VecRegContainer = ::DummyVecRegContainer;
--
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: 31
Gerrit-Owner: Gabe Black <gabe.bl...@gmail.com>
Gerrit-Reviewer: Andreas Sandberg <andreas.sandb...@arm.com>
Gerrit-Reviewer: Bobby R. Bruce <bbr...@ucdavis.edu>
Gerrit-Reviewer: Gabe Black <gabe.bl...@gmail.com>
Gerrit-Reviewer: Giacomo Travaglini <giacomo.travagl...@arm.com>
Gerrit-Reviewer: Tong Shen <endlessr...@google.com>
Gerrit-Reviewer: kokoro <noreply+kok...@google.com>
Gerrit-MessageType: merged
_______________________________________________
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
