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

Change subject: cpu: Use a unique_ptr to manage cached disassembly.
......................................................................

cpu: Use a unique_ptr to manage cached disassembly.

This is a fairly minor improvement, but avoids having to manually keep
track of that string pointer.

Change-Id: Ic3d4ddd9445920a110b36ab0cd64ff2289cf0139
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/42967
Maintainer: Gabe Black <[email protected]>
Tested-by: kokoro <[email protected]>
Reviewed-by: Daniel Carvalho <[email protected]>
---
M src/arch/mips/isa/formats/branch.isa
M src/arch/power/insts/branch.cc
M src/cpu/static_inst.cc
M src/cpu/static_inst.hh
4 files changed, 14 insertions(+), 24 deletions(-)

Approvals:
  Daniel Carvalho: Looks good to me, approved
  Gabe Black: Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/arch/mips/isa/formats/branch.isa b/src/arch/mips/isa/formats/branch.isa
index 09bab47..963d31d 100644
--- a/src/arch/mips/isa/formats/branch.isa
+++ b/src/arch/mips/isa/formats/branch.isa
@@ -153,14 +153,11 @@
     PCDependentDisassembly::disassemble(
             Addr pc, const Loader::SymbolTable *symtab) const
     {
-        if (!cachedDisassembly ||
-            pc != cachedPC || symtab != cachedSymtab)
-        {
-            if (cachedDisassembly)
-                delete cachedDisassembly;
+ if (!cachedDisassembly || pc != cachedPC || symtab != cachedSymtab) {
+            if (!cachedDisassembly)
+                cachedDisassembly.reset(new std::string);

-            cachedDisassembly =
-                new std::string(generateDisassembly(pc, symtab));
+            *cachedDisassembly = generateDisassembly(pc, symtab);
             cachedPC = pc;
             cachedSymtab = symtab;
         }
diff --git a/src/arch/power/insts/branch.cc b/src/arch/power/insts/branch.cc
index 72e4412..84834f1 100644
--- a/src/arch/power/insts/branch.cc
+++ b/src/arch/power/insts/branch.cc
@@ -37,14 +37,11 @@
 PCDependentDisassembly::disassemble(
         Addr pc, const Loader::SymbolTable *symtab) const
 {
-    if (!cachedDisassembly ||
-        pc != cachedPC || symtab != cachedSymtab)
-    {
-        if (cachedDisassembly)
-            delete cachedDisassembly;
+    if (!cachedDisassembly || pc != cachedPC || symtab != cachedSymtab) {
+        if (!cachedDisassembly)
+            cachedDisassembly.reset(new std::string);

-        cachedDisassembly =
-            new std::string(generateDisassembly(pc, symtab));
+        *cachedDisassembly = generateDisassembly(pc, symtab);
         cachedPC = pc;
         cachedSymtab = symtab;
     }
diff --git a/src/cpu/static_inst.cc b/src/cpu/static_inst.cc
index 05cd360..c42b1d2 100644
--- a/src/cpu/static_inst.cc
+++ b/src/cpu/static_inst.cc
@@ -64,12 +64,6 @@
 StaticInstPtr StaticInst::nullStaticInstPtr;
 StaticInstPtr StaticInst::nopStaticInstPtr = new NopStaticInst;

-StaticInst::~StaticInst()
-{
-    if (cachedDisassembly)
-        delete cachedDisassembly;
-}
-
 bool
 StaticInst::hasBranchTarget(const TheISA::PCState &pc, ThreadContext *tc,
                             TheISA::PCState &tgt) const
@@ -111,8 +105,10 @@
 const std::string &
 StaticInst::disassemble(Addr pc, const Loader::SymbolTable *symtab) const
 {
-    if (!cachedDisassembly)
- cachedDisassembly = new std::string(generateDisassembly(pc, symtab));
+    if (!cachedDisassembly) {
+        cachedDisassembly =
+            std::make_unique<std::string>(generateDisassembly(pc, symtab));
+    }

     return *cachedDisassembly;
 }
diff --git a/src/cpu/static_inst.hh b/src/cpu/static_inst.hh
index ebc5529..20411e7 100644
--- a/src/cpu/static_inst.hh
+++ b/src/cpu/static_inst.hh
@@ -283,7 +283,7 @@
      * String representation of disassembly (lazily evaluated via
      * disassemble()).
      */
-    mutable std::string *cachedDisassembly = nullptr;
+    mutable std::unique_ptr<std::string> cachedDisassembly;

     /**
      * Internal function to generate disassembly string.
@@ -301,7 +301,7 @@
     {}

   public:
-    virtual ~StaticInst();
+    virtual ~StaticInst() {};

     virtual Fault execute(ExecContext *xc,
             Trace::InstRecord *traceData) const = 0;



2 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/42967
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: Ic3d4ddd9445920a110b36ab0cd64ff2289cf0139
Gerrit-Change-Number: 42967
Gerrit-PatchSet: 4
Gerrit-Owner: Gabe Black <[email protected]>
Gerrit-Reviewer: Daniel Carvalho <[email protected]>
Gerrit-Reviewer: Gabe Black <[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