Andreas Sandberg has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/34118 )

Change subject: base: Cleanup debug flags API
......................................................................

base: Cleanup debug flags API

The debug flags API has a couple of quirks that should be cleaned
up. Specifically:

 * Only CompoundFlag should expose a list of children.
 * The global enable flag is just called "active", this isn't very
   descriptive.
 * Only SimpleFlag exposed a status member. This should be in the base
   class to make the API symmetric.
 * Flag::Sync() is an implementation detail and needs to be protected.

Change-Id: I4d7fd32c80891191aa04f0bd0c334c8cf8d372f5
Signed-off-by: Andreas Sandberg <andreas.sandb...@arm.com>
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/34118
Reviewed-by: Jason Lowe-Power <power...@gmail.com>
Maintainer: Jason Lowe-Power <power...@gmail.com>
Tested-by: kokoro <noreply+kok...@google.com>
---
M src/base/debug.cc
M src/base/debug.hh
M src/base/trace.cc
M src/python/m5/debug.py
M src/python/pybind11/debug.cc
5 files changed, 79 insertions(+), 44 deletions(-)

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



diff --git a/src/base/debug.cc b/src/base/debug.cc
index 47febd0..45d9f9d 100644
--- a/src/base/debug.cc
+++ b/src/base/debug.cc
@@ -1,4 +1,16 @@
 /*
+ * Copyright (c) 2020 ARM Limited
+ * All rights reserved
+ *
+ * The license below extends only to copyright in the software and shall
+ * not be construed as granting a license to any other intellectual
+ * property including but not limited to intellectual property relating
+ * to a hardware implementation of the functionality of the software
+ * licensed hereunder.  You may use the software subject to the license
+ * terms below provided that you ensure that this notice is replicated
+ * unmodified and in its entirety in all distributions of the software,
+ * modified or unmodified, in source code or in binary form.
+ *
  * Copyright (c) 2003-2005 The Regents of The University of Michigan
  * All rights reserved.
  *
@@ -67,7 +79,7 @@
     return flags;
 }

-bool SimpleFlag::_active = false;
+bool Flag::_globalEnable = false;

 Flag *
 findFlag(const std::string &name)
@@ -96,17 +108,17 @@
 }

 void
-SimpleFlag::enableAll()
+Flag::globalEnable()
 {
-    _active = true;
+    _globalEnable = true;
     for (auto& i : allFlags())
         i.second->sync();
 }

 void
-SimpleFlag::disableAll()
+Flag::globalDisable()
 {
-    _active = false;
+    _globalEnable = false;
     for (auto& i : allFlags())
         i.second->sync();
 }
@@ -125,6 +137,19 @@
         k->disable();
 }

+bool
+CompoundFlag::status() const
+{
+    if (_kids.empty())
+        return false;
+
+    for (auto& k : _kids) {
+        if (!k->status())
+            return false;
+    }
+
+    return true;
+}

 bool
 changeFlag(const char *s, bool value)
diff --git a/src/base/debug.hh b/src/base/debug.hh
index 1d35be0..a5dc43c 100644
--- a/src/base/debug.hh
+++ b/src/base/debug.hh
@@ -1,4 +1,16 @@
 /*
+ * Copyright (c) 2020 ARM Limited
+ * All rights reserved
+ *
+ * The license below extends only to copyright in the software and shall
+ * not be construed as granting a license to any other intellectual
+ * property including but not limited to intellectual property relating
+ * to a hardware implementation of the functionality of the software
+ * licensed hereunder.  You may use the software subject to the license
+ * terms below provided that you ensure that this notice is replicated
+ * unmodified and in its entirety in all distributions of the software,
+ * modified or unmodified, in source code or in binary form.
+ *
  * Copyright (c) 2003-2005 The Regents of The University of Michigan
  * Copyright (c) 2010 The Hewlett-Packard Development Company
  * All rights reserved.
@@ -42,45 +54,48 @@
 class Flag
 {
   protected:
+    static bool _globalEnable; // whether debug tracings are enabled
+
     const char *_name;
     const char *_desc;

+    virtual void sync() { }
+
   public:
     Flag(const char *name, const char *desc);
     virtual ~Flag();

     std::string name() const { return _name; }
     std::string desc() const { return _desc; }
-    virtual std::vector<Flag *> kids() { return std::vector<Flag*>(); }

     virtual void enable() = 0;
     virtual void disable() = 0;
-    virtual void sync() {}
+    virtual bool status() const = 0;
+
+    operator bool() const { return status(); }
+    bool operator!() const { return !status(); }
+
+    static void globalEnable();
+    static void globalDisable();
 };

 class SimpleFlag : public Flag
 {
-    static bool _active; // whether debug tracings are enabled
   protected:
     bool _tracing; // tracing is enabled and flag is on
     bool _status;  // flag status

+    void sync() override { _tracing = _globalEnable && _status; }
+
   public:
     SimpleFlag(const char *name, const char *desc)
         : Flag(name, desc), _status(false)
     { }

-    bool status() const { return _tracing; }
-    operator bool() const { return _tracing; }
-    bool operator!() const { return !_tracing; }
+    bool status() const override { return _tracing; }

-    void enable()  { _status = true;  sync(); }
-    void disable() { _status = false; sync(); }
-
-    void sync() { _tracing = _active && _status; }
-
-    static void enableAll();
-    static void disableAll();
+    void enable() override  { _status = true;  sync(); }
+    void disable() override { _status = false; sync(); }
 };

 class CompoundFlag : public Flag
@@ -97,10 +112,11 @@
     {
     }

-    std::vector<Flag *> kids() { return _kids; }
+    const std::vector<Flag *> &kids() const { return _kids; }

-    void enable();
-    void disable();
+    void enable() override;
+    void disable() override;
+    bool status() const override;
 };

 typedef std::map<std::string, Flag *> FlagsMap;
diff --git a/src/base/trace.cc b/src/base/trace.cc
index 8f97a94..ed6fbd2 100644
--- a/src/base/trace.cc
+++ b/src/base/trace.cc
@@ -91,13 +91,13 @@
 void
 enable()
 {
-    Debug::SimpleFlag::enableAll();
+    Debug::Flag::globalEnable();
 }

 void
 disable()
 {
-    Debug::SimpleFlag::disableAll();
+    Debug::Flag::globalDisable();
 }

 ObjectMatch ignore;
diff --git a/src/python/m5/debug.py b/src/python/m5/debug.py
index a3d5e35..6b45b16 100644
--- a/src/python/m5/debug.py
+++ b/src/python/m5/debug.py
@@ -34,24 +34,18 @@
 from m5.util import printList

 def help():
+    sorted_flags = sorted(flags.items(), key=lambda kv: kv[0])
+
     print("Base Flags:")
-    for name in sorted(flags):
-        if name == 'All':
-            continue
-        flag = flags[name]
-        children = [c for c in flag.kids() ]
-        if not children:
-            print("    %s: %s" % (name, flag.desc()))
+ for name, flag in filter(lambda kv: not isinstance(kv[1], CompoundFlag),
+                             sorted_flags):
+        print("    %s: %s" % (name, flag.desc))
     print()
     print("Compound Flags:")
-    for name in sorted(flags):
-        if name == 'All':
-            continue
-        flag = flags[name]
-        children = [c for c in flag.kids() ]
-        if children:
-            print("    %s: %s" % (name, flag.desc()))
-            printList([ c.name() for c in children ], indent=8)
+    for name, flag in filter(lambda kv: isinstance(kv[1], CompoundFlag),
+                             sorted_flags):
+        print("    %s: %s" % (name, flag.desc))
+        printList([ c.name for c in flag.kids() ], indent=8)
     print()

 class AllFlags(Mapping):
diff --git a/src/python/pybind11/debug.cc b/src/python/pybind11/debug.cc
index 766ccea..ed2942b 100644
--- a/src/python/pybind11/debug.cc
+++ b/src/python/pybind11/debug.cc
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017 ARM Limited
+ * Copyright (c) 2017, 2020 ARM Limited
  * All rights reserved
  *
  * The license below extends only to copyright in the software and shall
@@ -94,16 +94,16 @@

     py::class_<Debug::Flag> c_flag(m_debug, "Flag");
     c_flag
-        .def("name", &Debug::Flag::name)
-        .def("desc", &Debug::Flag::desc)
-        .def("kids", &Debug::Flag::kids)
+        .def_property_readonly("name", &Debug::Flag::name)
+        .def_property_readonly("desc", &Debug::Flag::desc)
         .def("enable", &Debug::Flag::enable)
         .def("disable", &Debug::Flag::disable)
-        .def("sync", &Debug::Flag::sync)
         ;

     py::class_<Debug::SimpleFlag>(m_debug, "SimpleFlag", c_flag);
-    py::class_<Debug::CompoundFlag>(m_debug, "CompoundFlag", c_flag);
+    py::class_<Debug::CompoundFlag>(m_debug, "CompoundFlag", c_flag)
+        .def("kids", &Debug::CompoundFlag::kids)
+        ;


     py::module m_trace = m_native.def_submodule("trace");

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/34118
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: I4d7fd32c80891191aa04f0bd0c334c8cf8d372f5
Gerrit-Change-Number: 34118
Gerrit-PatchSet: 2
Gerrit-Owner: Andreas Sandberg <andreas.sandb...@arm.com>
Gerrit-Reviewer: Andreas Sandberg <andreas.sandb...@arm.com>
Gerrit-Reviewer: Bobby R. Bruce <bbr...@ucdavis.edu>
Gerrit-Reviewer: Jason Lowe-Power <ja...@lowepower.com>
Gerrit-Reviewer: Jason Lowe-Power <power...@gmail.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

Reply via email to