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.sandberg@arm.com>
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/34118
Reviewed-by: Jason Lowe-Power <power.jg@gmail.com>
Maintainer: Jason Lowe-Power <power.jg@gmail.com>
Tested-by: kokoro <noreply+kokoro@google.com>

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 @@ allFlags()
     return flags;
 }
 
-bool SimpleFlag::_active = false;
+bool Flag::_globalEnable = false;
 
 Flag *
 findFlag(const std::string &name)
@@ -96,17 +108,17 @@ Flag::~Flag()
 }
 
 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 @@ CompoundFlag::disable()
         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)

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 @@ void breakpoint();
 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 @@ class CompoundFlag : public Flag
     {
     }
 
-    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;

src/base/trace.cc

@@ -91,13 +91,13 @@ setDebugLogger(Logger *logger)
 void
 enable()
 {
-    Debug::SimpleFlag::enableAll();
+    Debug::Flag::globalEnable();
 }
 
 void
 disable()
 {
-    Debug::SimpleFlag::disableAll();
+    Debug::Flag::globalDisable();
 }
 
 ObjectMatch ignore;

src/python/m5/debug.py

@@ -34,24 +34,18 @@ from _m5.debug import schedBreak, setRemoteGDBPort
 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):

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 @@ pybind_init_debug(py::module &m_native)
 
     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");