derek/gem5
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

base,python: Simplify how we check if a debug flag is enabled.

Browse Source 
Compound debug flags are intended to be a way to enable or disable a
group of simple debug flags at once, so that you don't need to enumerate
every more specialized flag in an area to get a broad amount of
debugging, nor do you give up the ability to select a general area
easily by making more specific flags.

It doesn't, however, make a lot of sense to check the value of a
compound debug flag, since it could be enabled but then have individual
subflags disabled. Exactly whether that means the compound flag should
be enabled or not is not clear, and figuring it out takes a fair amount
of work since each member simple flag needs to be visited.

Also, by having different behavior depending on the flag type, the
"enabled" method needed to be virtual.

This change eliminates the virtual method and moves the _tracing bool
member into the base class. If a subclass (only SimpleFlag currently)
wants to start or stop tracing based on itself, it should set or clear
this flag. Also, the "enabled" method has been renamed to "tracing",
since that's actually what it tracked. Being enabled by itself is not
sufficient to be tracing since there is also a global enable.

Finally, rather than duplicate the logic to convert a flag to bool in
the python wrapper, we can just use a cast to bool and take advantage of
the version in C++.

Change-Id: I3dc64c2364f0239294093686ddac6fcc8441f306
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/45007
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Gabe Black <gabe.black@gmail.com>
Maintainer: Gabe Black <gabe.black@gmail.com>
This commit is contained in:
Gabe Black
2021-04-29 21:58:48 -07:00
parent 35b6961dcb
commit 429e53a600
4 changed files with 64 additions and 81 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
src/base/debug.cc
View File

@@ -136,20 +136,6 @@ CompoundFlag::disable()
k->disable();
}
bool
CompoundFlag::enabled() const
{
if (_kids.empty())
return false;
for (auto& k : _kids) {
if (!k->enabled())
return false;
}
return true;
}
bool
changeFlag(const char *s, bool value)
{
@@ -188,7 +174,7 @@ dumpDebugFlags(std::ostream &os)
FlagsMap::iterator end = allFlags().end();
for (; i != end; ++i) {
SimpleFlag *f = dynamic_cast<SimpleFlag *>(i->second);
if (f && f->enabled())
if (f && f->tracing())
ccprintf(os, "%s\n", f->name());
}
}

11
src/base/debug.hh
View File

@@ -57,6 +57,8 @@ class Flag
protected:
static bool _globalEnable; // whether debug tracings are enabled
bool _tracing = false; // tracing is enabled and flag is on
const char *_name;
const char *_desc;
@@ -69,11 +71,12 @@ class Flag
std::string name() const { return _name; }
std::string desc() const { return _desc; }
bool tracing() const { return _tracing; }
virtual void enable() = 0;
virtual void disable() = 0;
virtual bool enabled() const = 0;
operator bool() const { return enabled(); }
operator bool() const { return tracing(); }
static void globalEnable();
static void globalDisable();
@@ -85,7 +88,6 @@ class SimpleFlag : public Flag
/** Whether this flag changes debug formatting. */
const bool _isFormat = false;
bool _tracing = false; // tracing is enabled and flag is on
bool _enabled = false; // flag enablement status
void sync() override { _tracing = _globalEnable && _enabled; }
@@ -95,8 +97,6 @@ class SimpleFlag : public Flag
: Flag(name, desc), _isFormat(is_format)
{}
bool enabled() const override { return _tracing; }
void enable() override { _enabled = true; sync(); }
void disable() override { _enabled = false; sync(); }
@@ -127,7 +127,6 @@ class CompoundFlag : public Flag
void enable() override;
void disable() override;
bool enabled() const override;
};
typedef std::map<std::string, Flag *> FlagsMap;

112
src/base/debug.test.cc
View File

@@ -78,23 +78,23 @@ TEST(DebugSimpleFlagTest, Enabled)
Debug::SimpleFlag flag("SimpleFlagEnabledTest", "");
// By default flags are initialized disabled
ASSERT_FALSE(flag.enabled());
ASSERT_FALSE(flag.tracing());
// Flags must be globally enabled before individual flags are enabled
flag.enable();
ASSERT_FALSE(flag.enabled());
ASSERT_FALSE(flag.tracing());
Debug::Flag::globalEnable();
ASSERT_TRUE(flag.enabled());
ASSERT_TRUE(flag.tracing());
// Verify that the global enabler works
Debug::Flag::globalDisable();
ASSERT_FALSE(flag.enabled());
ASSERT_FALSE(flag.tracing());
Debug::Flag::globalEnable();
ASSERT_TRUE(flag.enabled());
ASSERT_TRUE(flag.tracing());
// Test disabling the flag with global enabled
flag.disable();
ASSERT_FALSE(flag.enabled());
ASSERT_FALSE(flag.tracing());
}
/**
@@ -110,29 +110,28 @@ TEST(DebugCompoundFlagTest, Enabled)
{&flag_a, &flag_b});
// By default flags are initialized disabled
ASSERT_FALSE(flag.enabled());
ASSERT_FALSE(flag.tracing());
// Flags must be globally enabled before individual flags are enabled
flag.enable();
ASSERT_FALSE(flag_a.enabled());
ASSERT_FALSE(flag_b.enabled());
ASSERT_FALSE(flag.enabled());
ASSERT_FALSE(flag_a.tracing());
ASSERT_FALSE(flag_b.tracing());
ASSERT_FALSE(flag.tracing());
Debug::Flag::globalEnable();
for (auto &kid : flag.kids()) {
ASSERT_TRUE(kid->enabled());
ASSERT_TRUE(kid->tracing());
}
ASSERT_TRUE(flag_a.enabled());
ASSERT_TRUE(flag_b.enabled());
ASSERT_TRUE(flag.enabled());
ASSERT_TRUE(flag_a.tracing());
ASSERT_TRUE(flag_b.tracing());
// Test disabling the flag with global enabled
flag.disable();
for (auto &kid : flag.kids()) {
ASSERT_FALSE(kid->enabled());
ASSERT_FALSE(kid->tracing());
}
ASSERT_FALSE(flag_a.enabled());
ASSERT_FALSE(flag_b.enabled());
ASSERT_FALSE(flag.enabled());
ASSERT_FALSE(flag_a.tracing());
ASSERT_FALSE(flag_b.tracing());
ASSERT_FALSE(flag.tracing());
}
/** Test that the conversion operator matches the enablement status. */
@@ -141,9 +140,9 @@ TEST(DebugFlagTest, ConversionOperator)
Debug::Flag::globalEnable();
Debug::SimpleFlag flag("FlagConversionOperatorTest", "");
ASSERT_EQ(flag, flag.enabled());
ASSERT_EQ(flag, flag.tracing());
flag.enable();
ASSERT_EQ(flag, flag.enabled());
ASSERT_EQ(flag, flag.tracing());
flag.disable();
}
@@ -160,28 +159,27 @@ TEST(DebugCompoundFlagTest, EnabledKids)
{&flag_a, &flag_b});
// Test enabling only flag A
ASSERT_FALSE(flag_a.enabled());
ASSERT_FALSE(flag_b.enabled());
ASSERT_FALSE(flag.enabled());
ASSERT_FALSE(flag_a.tracing());
ASSERT_FALSE(flag_b.tracing());
ASSERT_FALSE(flag.tracing());
flag_a.enable();
ASSERT_TRUE(flag_a.enabled());
ASSERT_FALSE(flag_b.enabled());
ASSERT_FALSE(flag.enabled());
ASSERT_TRUE(flag_a.tracing());
ASSERT_FALSE(flag_b.tracing());
ASSERT_FALSE(flag.tracing());
// Test that enabling both flags enables the compound flag
ASSERT_TRUE(flag_a.enabled());
ASSERT_FALSE(flag_b.enabled());
ASSERT_FALSE(flag.enabled());
ASSERT_TRUE(flag_a.tracing());
ASSERT_FALSE(flag_b.tracing());
ASSERT_FALSE(flag.tracing());
flag_b.enable();
ASSERT_TRUE(flag_a.enabled());
ASSERT_TRUE(flag_b.enabled());
ASSERT_TRUE(flag.enabled());
ASSERT_TRUE(flag_a.tracing());
ASSERT_TRUE(flag_b.tracing());
// Test that disabling one of the flags disables the compound flag
flag_a.disable();
ASSERT_FALSE(flag_a.enabled());
ASSERT_TRUE(flag_b.enabled());
ASSERT_FALSE(flag.enabled());
ASSERT_FALSE(flag_a.tracing());
ASSERT_TRUE(flag_b.tracing());
ASSERT_FALSE(flag.tracing());
}
/** Search for existent and non-existent flags. */
@@ -195,13 +193,13 @@ TEST(DebugFlagTest, FindFlag)
// enabled too
Debug::Flag *flag;
EXPECT_TRUE(flag = Debug::findFlag("FlagFindFlagTestA"));
ASSERT_FALSE(flag_a.enabled());
ASSERT_FALSE(flag_a.tracing());
flag->enable();
ASSERT_TRUE(flag_a.enabled());
ASSERT_TRUE(flag_a.tracing());
EXPECT_TRUE(flag = Debug::findFlag("FlagFindFlagTestB"));
ASSERT_FALSE(flag_b.enabled());
ASSERT_FALSE(flag_b.tracing());
flag->enable();
ASSERT_TRUE(flag_b.enabled());
ASSERT_TRUE(flag_b.tracing());
// Search for a non-existent flag
EXPECT_FALSE(Debug::findFlag("FlagFindFlagTestC"));
@@ -216,18 +214,18 @@ TEST(DebugFlagTest, ChangeFlag)
// Enable the found flags and verify that the original flags are
// enabled too
ASSERT_FALSE(flag_a.enabled());
ASSERT_FALSE(flag_a.tracing());
EXPECT_TRUE(Debug::changeFlag("FlagChangeFlagTestA", true));
ASSERT_TRUE(flag_a.enabled());
ASSERT_TRUE(flag_a.tracing());
EXPECT_TRUE(Debug::changeFlag("FlagChangeFlagTestA", false));
ASSERT_FALSE(flag_a.enabled());
ASSERT_FALSE(flag_a.tracing());
// Disable and enable a flag
ASSERT_FALSE(flag_b.enabled());
ASSERT_FALSE(flag_b.tracing());
EXPECT_TRUE(Debug::changeFlag("FlagChangeFlagTestB", false));
ASSERT_FALSE(flag_b.enabled());
ASSERT_FALSE(flag_b.tracing());
EXPECT_TRUE(Debug::changeFlag("FlagChangeFlagTestB", true));
ASSERT_TRUE(flag_b.enabled());
ASSERT_TRUE(flag_b.tracing());
// Change a non-existent flag
ASSERT_FALSE(Debug::changeFlag("FlagChangeFlagTestC", true));
@@ -241,18 +239,18 @@ TEST(DebugFlagTest, SetClearDebugFlag)
Debug::SimpleFlag flag_b("FlagSetClearDebugFlagTestB", "");
// Enable and disable a flag
ASSERT_FALSE(flag_a.enabled());
ASSERT_FALSE(flag_a.tracing());
setDebugFlag("FlagSetClearDebugFlagTestA");
ASSERT_TRUE(flag_a.enabled());
ASSERT_TRUE(flag_a.tracing());
clearDebugFlag("FlagSetClearDebugFlagTestA");
ASSERT_FALSE(flag_a.enabled());
ASSERT_FALSE(flag_a.tracing());
// Disable and enable a flag
ASSERT_FALSE(flag_b.enabled());
ASSERT_FALSE(flag_b.tracing());
clearDebugFlag("FlagSetClearDebugFlagTestB");
ASSERT_FALSE(flag_b.enabled());
ASSERT_FALSE(flag_b.tracing());
setDebugFlag("FlagSetClearDebugFlagTestB");
ASSERT_TRUE(flag_b.enabled());
ASSERT_TRUE(flag_b.tracing());
// Change a non-existent flag
setDebugFlag("FlagSetClearDebugFlagTestC");
@@ -270,7 +268,7 @@ TEST(DebugFlagTest, NoDumpDebugFlags)
dumpDebugFlags();
std::string output = gtestLogOutput.str();
EXPECT_EQ(output, "");
ASSERT_FALSE(flag.enabled());
ASSERT_FALSE(flag.tracing());
}
/** Test dumping enabled debug flags with a larger set of flags. */
@@ -288,11 +286,11 @@ TEST(DebugFlagTest, DumpDebugFlags)
{&flag_e});
// Enable a few flags
ASSERT_FALSE(flag_a.enabled());
ASSERT_FALSE(flag_b.enabled());
ASSERT_FALSE(flag_c.enabled());
ASSERT_FALSE(flag_d.enabled());
ASSERT_FALSE(flag_e.enabled());
ASSERT_FALSE(flag_a.tracing());
ASSERT_FALSE(flag_b.tracing());
ASSERT_FALSE(flag_c.tracing());
ASSERT_FALSE(flag_d.tracing());
ASSERT_FALSE(flag_e.tracing());
flag_a.enable();
flag_c.enable();
compound_flag_b.enable();

6
src/python/pybind11/debug.cc
View File

@@ -94,9 +94,9 @@ pybind_init_debug(py::module_ &m_native)
.def_property_readonly("desc", &Debug::Flag::desc)
.def("enable", &Debug::Flag::enable)
.def("disable", &Debug::Flag::disable)
.def_property("enabled",
.def_property("tracing",
[](const Debug::Flag *flag) {
return flag->enabled();
return flag->tracing();
},
[](Debug::Flag *flag, bool state) {
if (state) {
@@ -106,7 +106,7 @@ pybind_init_debug(py::module_ &m_native)
}
})
.def("__bool__", [](const Debug::Flag *flag) {
return flag->enabled();
return (bool)*flag;
})
;