Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/56906 )
Change subject: arch,arch-x86,cpu: Stop using macroop context for ROM uops.
......................................................................
arch,arch-x86,cpu: Stop using macroop context for ROM uops.
Using the current macroop to contextualize the ROM microops causes a few
problems, adds some complexity, and turns out to be less convenient, so
lets stop doing it.
First, when caching microops, the decoder actually caches entire
macroops which microops can then be read from. For ROM based microops
though, the decoder doesn't create and cache a new version of the ROM
which contains them all. Because the same micropc and the same ROM
needed to be able to return a contextualized microop, it would not
actually store them and would generate a new one every time.
Now that the uops are not contextualized based on the instruction and
are instead contextualized based on the default state for instructions
currently in effect, the decoder can cache them using the same scheme as
macroops.
Second, since ROM microcode would sometimes run with generic context and
sometimes run with the context of an instruction, it was difficult to
write that microcode since it was hard to reason about what settings
like operand size would be. Those could either be unusable junk, or what
you'd want to use.
Third, the CPUs would have to feed the current macroop into the decoder
when fetching a ROM microop. This added a (small) amount of complexity.
Fourth, because the ROM microops now see the live default state of the
CPU, they can change that state and then operate under it. This will
significantly simplify that sort of code, which now has to try to
determine what sizes to use, and then switch to a bank of microops which
implement those settings since dataSize, operandSize, etc, overrides have
to be hard coded.
Change-Id: I8f416e4d7b6c48790b2f5633aa68587823b1811c
---
M src/arch/generic/decoder.cc
M src/arch/generic/decoder.hh
M src/arch/x86/decoder.cc
M src/arch/x86/decoder.hh
M src/arch/x86/microcode_rom.hh
M src/cpu/checker/cpu_impl.hh
M src/cpu/o3/fetch.cc
M src/cpu/simple/base.cc
8 files changed, 94 insertions(+), 41 deletions(-)
diff --git a/src/arch/generic/decoder.cc b/src/arch/generic/decoder.cc
index 22e4d53..acd21f7 100644
--- a/src/arch/generic/decoder.cc
+++ b/src/arch/generic/decoder.cc
@@ -33,7 +33,7 @@
{
StaticInstPtr
-InstDecoder::fetchRomMicroop(MicroPC micropc, StaticInstPtr curMacroop)
+InstDecoder::fetchRomMicroop(MicroPC micropc)
{
panic("ROM based microcode isn't implemented.");
}
diff --git a/src/arch/generic/decoder.hh b/src/arch/generic/decoder.hh
index afba1a3..e49ca13 100644
--- a/src/arch/generic/decoder.hh
+++ b/src/arch/generic/decoder.hh
@@ -57,8 +57,7 @@
_pcMask(~mask(floorLog2(_moreBytesSize)))
{}
- virtual StaticInstPtr fetchRomMicroop(
- MicroPC micropc, StaticInstPtr curMacroop);
+ virtual StaticInstPtr fetchRomMicroop(MicroPC micropc);
virtual void
reset()
{
diff --git a/src/arch/x86/decoder.cc b/src/arch/x86/decoder.cc
index cc9373f..a99e452 100644
--- a/src/arch/x86/decoder.cc
+++ b/src/arch/x86/decoder.cc
@@ -672,6 +672,8 @@
Decoder::InstBytes Decoder::dummy;
Decoder::InstCacheMap Decoder::instCacheMap;
+Decoder::RomUopCacheMap Decoder::romUopCacheMap;
+
StaticInstPtr
Decoder::decode(ExtMachInst mach_inst, Addr addr)
{
@@ -735,10 +737,14 @@
}
StaticInstPtr
-Decoder::fetchRomMicroop(MicroPC micropc, StaticInstPtr curMacroop)
+Decoder::fetchRomMicroop(MicroPC micropc)
{
static const auto &rom = X86ISAInst::microcodeRom();
- return rom.fetchMicroop(micropc, curMacroop);
+ assert(romUopCache);
+ auto &rom_uop = (*romUopCache)[micropc];
+ if (!rom_uop)
+ rom_uop = rom.fetchMicroop(micropc, romEmi, romEmulEnv);
+ return rom_uop;
}
} // namespace X86ISA
diff --git a/src/arch/x86/decoder.hh b/src/arch/x86/decoder.hh
index f3dd870..473bc65 100644
--- a/src/arch/x86/decoder.hh
+++ b/src/arch/x86/decoder.hh
@@ -34,6 +34,7 @@
#include <vector>
#include "arch/generic/decoder.hh"
+#include "arch/x86/emulenv.hh"
#include "arch/x86/microcode_rom.hh"
#include "arch/x86/regs/misc.hh"
#include "arch/x86/types.hh"
@@ -110,6 +111,10 @@
uint8_t cpl = 0;
+ // State to use to generate ROM based microops.
+ ExtMachInst romEmi;
+ EmulEnv romEmulEnv;
+
uint8_t
getNextByte()
{
@@ -243,6 +248,11 @@
CacheKey, decode_cache::InstMap<ExtMachInst> *> InstCacheMap;
static InstCacheMap instCacheMap;
+ typedef std::unordered_map<MicroPC, StaticInstPtr> RomUopCache;
+ RomUopCache *romUopCache = nullptr;
+ typedef std::unordered_map<CacheKey, RomUopCache *> RomUopCacheMap;
+ static RomUopCacheMap romUopCacheMap;
+
StaticInstPtr decodeInst(ExtMachInst mach_inst);
/// Decode a machine instruction.
@@ -253,12 +263,15 @@
void process();
public:
- Decoder(const X86DecoderParams &p) : InstDecoder(p, &fetchChunk)
+ Decoder(const X86DecoderParams &p) : InstDecoder(p, &fetchChunk),
+ romEmulEnv(INTREG_T0, INTREG_T0, 0, 0, 0)
{
emi.reset();
emi.mode.cpl = cpl;
emi.mode.mode = mode;
emi.mode.submode = submode;
+
+ romEmi = emi;
}
void
@@ -276,21 +289,26 @@
defAddr = m5Reg.defAddr;
stack = m5Reg.stack;
- AddrCacheMap::iterator amIter = addrCacheMap.find(m5Reg);
- if (amIter != addrCacheMap.end()) {
- decodePages = amIter->second;
- } else {
- decodePages = new DecodePages;
- addrCacheMap[m5Reg] = decodePages;
- }
+ romEmi.mode = emi.mode;
- InstCacheMap::iterator imIter = instCacheMap.find(m5Reg);
- if (imIter != instCacheMap.end()) {
- instMap = imIter->second;
- } else {
- instMap = new decode_cache::InstMap<ExtMachInst>;
- instCacheMap[m5Reg] = instMap;
- }
+ auto &am_ptr = addrCacheMap[m5Reg];
+ if (!am_ptr)
+ am_ptr = new DecodePages;
+ decodePages = am_ptr;
+
+ auto &im_ptr = instCacheMap[m5Reg];
+ if (!im_ptr)
+ im_ptr = new decode_cache::InstMap<ExtMachInst>;
+ instMap = im_ptr;
+
+ auto &rom_ptr = romUopCacheMap[m5Reg];
+ if (!rom_ptr)
+ rom_ptr = new RomUopCache;
+ romUopCache = rom_ptr;
+
+ romEmulEnv.dataSize = 1 << defOp;
+ romEmulEnv.addressSize = 1 << defAddr;
+ romEmulEnv.stackSize = 1 << stack;
}
void
@@ -312,6 +330,9 @@
altAddr = dec->altAddr;
defAddr = dec->defAddr;
stack = dec->stack;
+
+ romEmi = dec->romEmi;
+ romEmulEnv = dec->romEmulEnv;
}
void
@@ -350,9 +371,7 @@
public:
StaticInstPtr decode(PCStateBase &next_pc) override;
-
- StaticInstPtr fetchRomMicroop(
- MicroPC micropc, StaticInstPtr curMacroop) override;
+ StaticInstPtr fetchRomMicroop(MicroPC micropc) override;
};
} // namespace X86ISA
diff --git a/src/arch/x86/microcode_rom.hh b/src/arch/x86/microcode_rom.hh
index c802a0a..3cd5848 100644
--- a/src/arch/x86/microcode_rom.hh
+++ b/src/arch/x86/microcode_rom.hh
@@ -76,24 +76,14 @@
}
StaticInstPtr
- fetchMicroop(MicroPC micro_pc, StaticInstPtr macroop) const
+ fetchMicroop(MicroPC micro_pc, const X86ISA::ExtMachInst &emi,
+ const X86ISA::EmulEnv &emul_env) const
{
micro_pc = normalMicroPC(micro_pc);
if (micro_pc >= genFuncs.size())
return X86ISA::badMicroop;
- static const X86ISA::ExtMachInst DummyExtMachInst = {};
- static const X86ISA::EmulEnv DummyEmulEnv(0, 0, 1, 1, 1);
-
- if (macroop) {
- auto *x86_macroop =
- static_cast<X86ISA::MacroopBase *>(macroop.get());
- return genFuncs[micro_pc]("Microcode_ROM", labels,
- x86_macroop->getExtMachInst(),
x86_macroop->getEmulEnv());
- } else {
- return genFuncs[micro_pc]("Microcode_ROM", labels,
- DummyExtMachInst, DummyEmulEnv);
- }
+ return genFuncs[micro_pc]("Microcode_ROM", labels, emi, emul_env);
}
};
diff --git a/src/cpu/checker/cpu_impl.hh b/src/cpu/checker/cpu_impl.hh
index 218d87f..e392562 100644
--- a/src/cpu/checker/cpu_impl.hh
+++ b/src/cpu/checker/cpu_impl.hh
@@ -286,7 +286,7 @@
if (isRomMicroPC(pc_state->microPC())) {
fetchDone = true;
curStaticInst = decoder->fetchRomMicroop(
- pc_state->microPC(), nullptr);
+ pc_state->microPC());
} else if (!curMacroStaticInst) {
//We're not in the middle of a macro instruction
StaticInstPtr instPtr = nullptr;
diff --git a/src/cpu/o3/fetch.cc b/src/cpu/o3/fetch.cc
index 5358b33..181bb7b 100644
--- a/src/cpu/o3/fetch.cc
+++ b/src/cpu/o3/fetch.cc
@@ -1262,8 +1262,7 @@
bool newMacro = false;
if (curMacroop || inRom) {
if (inRom) {
- staticInst = dec_ptr->fetchRomMicroop(
- this_pc.microPC(), curMacroop);
+ staticInst =
dec_ptr->fetchRomMicroop(this_pc.microPC());
} else {
staticInst =
curMacroop->fetchMicroop(this_pc.microPC());
}
diff --git a/src/cpu/simple/base.cc b/src/cpu/simple/base.cc
index 4cc8e1f..2a8a11c 100644
--- a/src/cpu/simple/base.cc
+++ b/src/cpu/simple/base.cc
@@ -322,8 +322,7 @@
if (isRomMicroPC(pc_state.microPC())) {
t_info.stayAtPC = false;
- curStaticInst = decoder->fetchRomMicroop(
- pc_state.microPC(), curMacroStaticInst);
+ curStaticInst = decoder->fetchRomMicroop(pc_state.microPC());
} else if (!curMacroStaticInst) {
//We're not in the middle of a macro instruction
StaticInstPtr instPtr = NULL;
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/56906
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: I8f416e4d7b6c48790b2f5633aa68587823b1811c
Gerrit-Change-Number: 56906
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
