Gabe Black has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/40099 )

Change subject: arch,cpu: Move a Decode DPRINTF into the arch Decoder classes.
......................................................................

arch,cpu: Move a Decode DPRINTF into the arch Decoder classes.

This DPRINTF accesses the ExtMachInst typed machInst member of the
StaticInst class, and so is ISA dependent. Move the DPRINTF to where the
instructions are actually decoded where that type doesn't have to be
disambiguated.

Also, this change makes this DPRINTF more accurate, since microops are
not really "decoded" when they are extracted from a macroop. The process
of unpacking them to feed into the rest of the CPU should be fairly
trivial, so really they're just being retrieved. With the DPRINTF in
this new position, it will only trigger when an instruction is actually
decoded from memory.

Change-Id: I14145165b93bb004057a729fa7909cd2d3d34d29
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/40099
Reviewed-by: Jason Lowe-Power <[email protected]>
Maintainer: Jason Lowe-Power <[email protected]>
Tested-by: kokoro <[email protected]>
---
M src/arch/arm/decoder.hh
M src/arch/mips/decoder.hh
M src/arch/power/decoder.hh
M src/arch/riscv/decoder.cc
M src/arch/sparc/decoder.hh
M src/arch/x86/decoder.cc
M src/cpu/simple/base.cc
7 files changed, 40 insertions(+), 19 deletions(-)

Approvals:
  Jason Lowe-Power: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/arch/arm/decoder.hh b/src/arch/arm/decoder.hh
index 4f8e71a..1f14328 100644
--- a/src/arch/arm/decoder.hh
+++ b/src/arch/arm/decoder.hh
@@ -49,6 +49,7 @@
 #include "arch/generic/decoder.hh"
 #include "base/types.hh"
 #include "cpu/static_inst.hh"
+#include "debug/Decode.hh"
 #include "enums/DecoderFlavor.hh"

 namespace ArmISA
@@ -172,7 +173,10 @@
     StaticInstPtr
     decode(ExtMachInst mach_inst, Addr addr)
     {
-        return defaultCache.decode(this, mach_inst, addr);
+        StaticInstPtr si = defaultCache.decode(this, mach_inst, addr);
+        DPRINTF(Decode, "Decode: Decoded %s instruction: %#x\n",
+                si->getName(), mach_inst);
+        return si;
     }

     /**
diff --git a/src/arch/mips/decoder.hh b/src/arch/mips/decoder.hh
index 9c6ae18..6e00bc3 100644
--- a/src/arch/mips/decoder.hh
+++ b/src/arch/mips/decoder.hh
@@ -35,6 +35,7 @@
 #include "base/logging.hh"
 #include "base/types.hh"
 #include "cpu/static_inst.hh"
+#include "debug/Decode.hh"

 namespace MipsISA
 {
@@ -98,7 +99,10 @@
     StaticInstPtr
     decode(ExtMachInst mach_inst, Addr addr)
     {
-        return defaultCache.decode(this, mach_inst, addr);
+        StaticInstPtr si = defaultCache.decode(this, mach_inst, addr);
+        DPRINTF(Decode, "Decode: Decoded %s instruction: %#x\n",
+                si->getName(), mach_inst);
+        return si;
     }

     StaticInstPtr
diff --git a/src/arch/power/decoder.hh b/src/arch/power/decoder.hh
index d89c9b1..4e02ef7 100644
--- a/src/arch/power/decoder.hh
+++ b/src/arch/power/decoder.hh
@@ -33,6 +33,7 @@
 #include "arch/generic/decoder.hh"
 #include "arch/power/types.hh"
 #include "cpu/static_inst.hh"
+#include "debug/Decode.hh"

 namespace PowerISA
 {
@@ -105,7 +106,10 @@
     StaticInstPtr
     decode(ExtMachInst mach_inst, Addr addr)
     {
-        return defaultCache.decode(this, mach_inst, addr);
+        StaticInstPtr si = defaultCache.decode(this, mach_inst, addr);
+        DPRINTF(Decode, "Decode: Decoded %s instruction: %#x\n",
+                si->getName(), mach_inst);
+        return si;
     }

     StaticInstPtr
diff --git a/src/arch/riscv/decoder.cc b/src/arch/riscv/decoder.cc
index a117991..26b6adb 100644
--- a/src/arch/riscv/decoder.cc
+++ b/src/arch/riscv/decoder.cc
@@ -81,13 +81,14 @@
 {
     DPRINTF(Decode, "Decoding instruction 0x%08x at address %#x\n",
             mach_inst, addr);
-    if (instMap.find(mach_inst) != instMap.end())
-        return instMap[mach_inst];
-    else {
-        StaticInstPtr si = decodeInst(mach_inst);
-        instMap[mach_inst] = si;
-        return si;
-    }
+
+    StaticInstPtr &si = instMap[mach_inst];
+    if (!si)
+        si = decodeInst(mach_inst);
+
+    DPRINTF(Decode, "Decode: Decoded %s instruction: %#x\n",
+            si->getName(), mach_inst);
+    return si;
 }

 StaticInstPtr
diff --git a/src/arch/sparc/decoder.hh b/src/arch/sparc/decoder.hh
index 8e68451..ece3b9c 100644
--- a/src/arch/sparc/decoder.hh
+++ b/src/arch/sparc/decoder.hh
@@ -34,6 +34,7 @@
 #include "arch/sparc/registers.hh"
 #include "arch/sparc/types.hh"
 #include "cpu/static_inst.hh"
+#include "debug/Decode.hh"

 namespace SparcISA
 {
@@ -112,7 +113,10 @@
     StaticInstPtr
     decode(ExtMachInst mach_inst, Addr addr)
     {
-        return defaultCache.decode(this, mach_inst, addr);
+        StaticInstPtr si = defaultCache.decode(this, mach_inst, addr);
+        DPRINTF(Decode, "Decode: Decoded %s instruction: %#x\n",
+                si->getName(), mach_inst);
+        return si;
     }

     StaticInstPtr
diff --git a/src/arch/x86/decoder.cc b/src/arch/x86/decoder.cc
index 415c7b4..95b80a8 100644
--- a/src/arch/x86/decoder.cc
+++ b/src/arch/x86/decoder.cc
@@ -32,6 +32,7 @@
 #include "base/logging.hh"
 #include "base/trace.hh"
 #include "base/types.hh"
+#include "debug/Decode.hh"
 #include "debug/Decoder.hh"

 namespace X86ISA
@@ -674,12 +675,18 @@
 StaticInstPtr
 Decoder::decode(ExtMachInst mach_inst, Addr addr)
 {
-    auto iter = instMap->find(mach_inst);
-    if (iter != instMap->end())
-        return iter->second;
+    StaticInstPtr si;

-    StaticInstPtr si = decodeInst(mach_inst);
-    (*instMap)[mach_inst] = si;
+    auto iter = instMap->find(mach_inst);
+    if (iter != instMap->end()) {
+        si = iter->second;
+    } else {
+        si = decodeInst(mach_inst);
+        (*instMap)[mach_inst] = si;
+    }
+
+    DPRINTF(Decode, "Decode: Decoded %s instruction: %#x\n",
+            si->getName(), mach_inst);
     return si;
 }

diff --git a/src/cpu/simple/base.cc b/src/cpu/simple/base.cc
index 62df848..0941388 100644
--- a/src/cpu/simple/base.cc
+++ b/src/cpu/simple/base.cc
@@ -368,9 +368,6 @@
 #if TRACING_ON
         traceData = tracer->getInstRecord(curTick(), thread->getTC(),
                 curStaticInst, thread->pcState(), curMacroStaticInst);
-
-        DPRINTF(Decode,"Decode: Decoded %s instruction: %#x\n",
-                curStaticInst->getName(), curStaticInst->machInst);
 #endif // TRACING_ON
     }


--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/40099
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: I14145165b93bb004057a729fa7909cd2d3d34d29
Gerrit-Change-Number: 40099
Gerrit-PatchSet: 7
Gerrit-Owner: Gabe Black <[email protected]>
Gerrit-Reviewer: Andreas Sandberg <[email protected]>
Gerrit-Reviewer: Bobby R. Bruce <[email protected]>
Gerrit-Reviewer: Gabe Black <[email protected]>
Gerrit-Reviewer: Giacomo Travaglini <[email protected]>
Gerrit-Reviewer: Jason Lowe-Power <[email protected]>
Gerrit-Reviewer: Jason Lowe-Power <[email protected]>
Gerrit-Reviewer: kokoro <[email protected]>
Gerrit-MessageType: merged
_______________________________________________
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