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

Change subject: arch-x86: Significantly simplify how ROM microops are generated.
......................................................................

arch-x86: Significantly simplify how ROM microops are generated.

Switch from lots of small functions to lambdas, and store and insert
dummy ExtMachInst and EmulEnv values from the MicrocodeRom class itself,
instead of in each small function. This will avoid having about a
thousand copies of these dummy values.

Change-Id: Ic5fc7924a4fb31a128ecf13e74969b62de1ad1cc
---
M src/arch/x86/isa/microops/base.isa
M src/arch/x86/isa/rom.isa
M src/arch/x86/microcode_rom.hh
3 files changed, 53 insertions(+), 81 deletions(-)



diff --git a/src/arch/x86/isa/microops/base.isa b/src/arch/x86/isa/microops/base.isa
index aded50b..6a5002b 100644
--- a/src/arch/x86/isa/microops/base.isa
+++ b/src/arch/x86/isa/microops/base.isa
@@ -199,27 +199,6 @@
         ImmType = Imm8Op

     class X86Microop(object):
-
-        generatorNameTemplate = "generate_%s_%d"
-
-        generatorTemplate = '''
-            StaticInstPtr
-            ''' + generatorNameTemplate + '''(StaticInstPtr curMacroop)
-            {
-                static const char *macrocodeBlock = romMnemonic;
-                static ExtMachInst dummyExtMachInst;
-                static const EmulEnv dummyEmulEnv(0, 0, 1, 1, 1);
-
- Macroop * macroop = dynamic_cast<Macroop *>(curMacroop.get());
-                const ExtMachInst &machInst =
-                    macroop ? macroop->getExtMachInst() : dummyExtMachInst;
-                [[maybe_unused]] const EmulEnv &env =
-                    macroop ? macroop->getEmulEnv() : dummyEmulEnv;
-                using namespace rom_labels;
-                return %s;
-            }
-        '''
-
         def __init__(self, name):
             self.name = name

@@ -227,11 +206,12 @@
             wrapped = ("(1ULL << StaticInst::%s)" % flag for flag in flags)
             return " | ".join(wrapped)

-        def getGeneratorDef(self, micropc):
-            return self.generatorTemplate % \
-                (self.className, micropc, \
-                 self.getAllocator(["IsMicroop", "IsDelayedCommit"]))
-
-        def getGenerator(self, micropc):
-            return self.generatorNameTemplate % (self.className, micropc)
+        def getGenerator(self):
+            return f'''
+            [](const char *macrocodeBlock, const ExtMachInst &machInst,
+                    const EmulEnv &env)
+            {'{'}
+                using namespace rom_labels;
+ return {self.getAllocator(["IsMicroop", "IsDelayedCommit"])};
+            {'}'}'''
 }};
diff --git a/src/arch/x86/isa/rom.isa b/src/arch/x86/isa/rom.isa
index 9aef3ba..93f4401 100644
--- a/src/arch/x86/isa/rom.isa
+++ b/src/arch/x86/isa/rom.isa
@@ -24,27 +24,12 @@
 // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

-def template MicroRomConstructor {{
-
-    %(define_generators)s
-    const MicroPC X86ISA::MicrocodeRom::numMicroops = %(num_microops)s;
-
-    X86ISA::MicrocodeRom::MicrocodeRom()
-    {
-        using namespace rom_labels;
-        genFuncs = new GenFunc[numMicroops];
-        %(alloc_generators)s;
-    }
-}};
-
 let {{
     from micro_asm import Rom

     class X86MicrocodeRom(Rom):
         def getDeclaration(self):
-            declareLabels = \
-                "GEM5_DEPRECATED_NAMESPACE(RomLabels, rom_labels);\n"
-            declareLabels += "namespace rom_labels\n{\n"
+            declareLabels = "namespace rom_labels\n{\n"
             for (label, microop) in self.labels.items():
                 declareLabels += "const static uint64_t label_%s = %d;\n" \
                                   % (label, microop.micropc)
@@ -56,25 +41,10 @@
             return declareLabels;

         def getDefinition(self):
-            numMicroops = len(self.microops)
-            allocGenerators = ''
-            micropc = 0
-            define_generators = '''
-                namespace
-                {
-                    static const char romMnemonic[] = "Microcode_ROM";
-            '''
-            for op in self.microops:
-                define_generators += op.getGeneratorDef(micropc)
-                allocGenerators += "genFuncs[%d] = %s;\n" % \
-                        (micropc, op.getGenerator(micropc))
-                micropc += 1
-            define_generators += "}\n"
-            iop = InstObjParams(self.name, self.name, "MicrocodeRom",
-                                {"code" : "",
-                                 "define_generators" : define_generators,
-                                 "num_microops" : numMicroops,
-                                 "alloc_generators" : allocGenerators
-                                })
-            return MicroRomConstructor.subst(iop);
+            elements = map(lambda op: op.getGenerator(), self.microops)
+            allocator = "{" + ',\n'.join(elements) + "}"
+
+            return '''
+    X86ISA::MicrocodeRom::MicrocodeRom() : genFuncs(%s) {}
+    ''' % allocator
 }};
diff --git a/src/arch/x86/microcode_rom.hh b/src/arch/x86/microcode_rom.hh
index 31693cf..1c95154 100644
--- a/src/arch/x86/microcode_rom.hh
+++ b/src/arch/x86/microcode_rom.hh
@@ -29,8 +29,12 @@
 #ifndef __ARCH_X86_MICROCODE_ROM_HH__
 #define __ARCH_X86_MICROCODE_ROM_HH__

-#include "arch/x86/insts/badmicroop.hh"
+#include <functional>
+#include <vector>
+
 #include "arch/x86/emulenv.hh"
+#include "arch/x86/insts/badmicroop.hh"
+#include "arch/x86/insts/macroop.hh"
 #include "cpu/static_inst.hh"

 namespace gem5
@@ -43,30 +47,34 @@
 {
   protected:

-    typedef StaticInstPtr (*GenFunc)(StaticInstPtr);
+    using GenFunc = std::function<StaticInstPtr(const char *macrocodeBlock,
+ const X86ISA::ExtMachInst &machInst, const X86ISA::EmulEnv &env)>;

-    static const MicroPC numMicroops;
-
-    GenFunc *genFuncs;
+    std::vector<GenFunc> genFuncs;

   public:
     //Constructor.
     MicrocodeRom();

-    //Destructor.
-    ~MicrocodeRom()
-    {
-        delete [] genFuncs;
-    }
-
     StaticInstPtr
-    fetchMicroop(MicroPC microPC, StaticInstPtr curMacroop)
+    fetchMicroop(MicroPC micro_pc, StaticInstPtr macroop)
     {
-        microPC = normalMicroPC(microPC);
-        if (microPC >= numMicroops)
+        micro_pc = normalMicroPC(micro_pc);
+        if (micro_pc >= genFuncs.size())
             return X86ISA::badMicroop;
-        else
-            return genFuncs[microPC](curMacroop);
+
+        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",
+ x86_macroop->getExtMachInst(), x86_macroop->getEmulEnv());
+        } else {
+            return genFuncs[micro_pc]("Microcode_ROM", DummyExtMachInst,
+                    DummyEmulEnv);
+        }
     }
 };


--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/56331
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: Ic5fc7924a4fb31a128ecf13e74969b62de1ad1cc
Gerrit-Change-Number: 56331
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black <gabe.bl...@gmail.com>
Gerrit-MessageType: newchange
_______________________________________________
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

Reply via email to