From 93ccc23b4ac40b92f4f7c310c17d798e686dcd71 Mon Sep 17 00:00:00 2001 From: Andreas Sandberg Date: Fri, 4 Sep 2020 16:01:24 +0100 Subject: [PATCH] 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 Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/34118 Reviewed-by: Jason Lowe-Power Maintainer: Jason Lowe-Power Tested-by: kokoro --- src/base/debug.cc | 35 ++++++++++++++++++++++---- src/base/debug.hh | 48 ++++++++++++++++++++++++------------ src/base/trace.cc | 4 +-- src/python/m5/debug.py | 24 +++++++----------- src/python/pybind11/debug.cc | 12 ++++----- 5 files changed, 79 insertions(+), 44 deletions(-) diff --git a/src/base/debug.cc b/src/base/debug.cc index 47febd0cd..45d9f9d59 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 @@ 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) diff --git a/src/base/debug.hh b/src/base/debug.hh index 1d35be055..a5dc43c43 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 @@ 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 kids() { return std::vector(); } 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; } - - void enable() { _status = true; sync(); } - void disable() { _status = false; sync(); } - - void sync() { _tracing = _active && _status; } + bool status() const override { return _tracing; } - 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 kids() { return _kids; } + const std::vector &kids() const { return _kids; } - void enable(); - void disable(); + void enable() override; + void disable() override; + bool status() const override; }; typedef std::map FlagsMap; diff --git a/src/base/trace.cc b/src/base/trace.cc index 8f97a9499..ed6fbd21a 100644 --- a/src/base/trace.cc +++ b/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; diff --git a/src/python/m5/debug.py b/src/python/m5/debug.py index a3d5e35ed..6b45b16ff 100644 --- a/src/python/m5/debug.py +++ b/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): diff --git a/src/python/pybind11/debug.cc b/src/python/pybind11/debug.cc index 766ccea7b..ed2942bc7 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 @@ pybind_init_debug(py::module &m_native) py::class_ 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_(m_debug, "SimpleFlag", c_flag); - py::class_(m_debug, "CompoundFlag", c_flag); + py::class_(m_debug, "CompoundFlag", c_flag) + .def("kids", &Debug::CompoundFlag::kids) + ; py::module m_trace = m_native.def_submodule("trace"); -- 2.30.2