Gabe Black has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/48370 )
1 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the
submitted one.
Change subject: scons,debug: Implement the "All" flag in C++ and not scons.
......................................................................
scons,debug: Implement the "All" flag in C++ and not scons.
Create an AllFlagsFlag class which inherits from the CompoundFlag class.
This class is a singleton, and the SimpleFlags install themselves in it
instead of having SCons collect them.
The allFlagsVersion global variable was supposed to be for debugging
according to a comment, but was actually an important part of the "All"
flags inner workings. It was not exposed in the header, but was
redefined/pulled through in src/python/pybind11/debug.cc. The
AllFlagsFlag class now tracks that value, and it can be accessed without
reaching behind the curtain.
This also somewhat decentralizes the debug flag building process in
SCons. The debug/flags.cc still includes all flags at once which
centralizes them, but at least now the "All" flag won't also.
Change-Id: I8430e0fe9022846aade028fb46c80777169a2007
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/48370
Tested-by: kokoro <[email protected]>
Reviewed-by: Daniel Carvalho <[email protected]>
Reviewed-by: Nathanael Premillieu <[email protected]>
Reviewed-by: Andreas Sandberg <[email protected]>
Maintainer: Andreas Sandberg <[email protected]>
---
M src/SConscript
M src/base/debug.cc
M src/base/debug.hh
M src/python/pybind11/debug.cc
4 files changed, 61 insertions(+), 33 deletions(-)
Approvals:
Andreas Sandberg: Looks good to me, approved; Looks good to me, approved
Daniel Carvalho: Looks good to me, but someone else must approve
Nathanael Premillieu: Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/SConscript b/src/SConscript
index 217df2c..838c3cd 100644
--- a/src/SConscript
+++ b/src/SConscript
@@ -594,13 +594,17 @@
#
debug_flags = {}
def DebugFlag(name, desc=None, fmt=False):
+ if name == "All":
+ raise AttributeError('The "All" flag name is reserved')
if name in debug_flags:
- raise AttributeError("Flag {} already specified".format(name))
+ raise AttributeError('Flag {} already specified'.format(name))
debug_flags[name] = (name, (), desc, fmt)
def CompoundFlag(name, flags, desc=None):
+ if name == "All":
+ raise AttributeError('The "All" flag name is reserved')
if name in debug_flags:
- raise AttributeError("Flag {} already specified".format(name))
+ raise AttributeError('Flag {} already specified'.format(name))
compound = tuple(flags)
debug_flags[name] = (name, compound, desc, False)
@@ -608,19 +612,6 @@
def DebugFormatFlag(name, desc=None):
DebugFlag(name, desc, True)
-# Create a compound debug flag that encapsulates all flags: "All". This
-# flag should not be used within C++ code - it is a compound meta flag
-def _createAllDebugFlag():
- simple_flags = []
- for name,flag in sorted(debug_flags.items()):
- n, compound, desc, fmt = flag
- assert n == name
- if not compound and not fmt:
- simple_flags.append(n)
-
- CompoundFlag("All", simple_flags,
- "Controls all debug flags. It should not be used within C++ code.")
-
Export('DebugFlag')
Export('CompoundFlag')
Export('DebugFormatFlag')
@@ -1156,7 +1147,6 @@
code.write(str(target[0]))
# Generate the files for the debug and debug-format flags
-_createAllDebugFlag()
for name,flag in sorted(debug_flags.items()):
n, compound, desc, fmt = flag
assert n == name
diff --git a/src/base/debug.cc b/src/base/debug.cc
index 8e65e1f..aa4092a 100644
--- a/src/base/debug.cc
+++ b/src/base/debug.cc
@@ -71,13 +71,17 @@
#endif
}
-//
-// Flags for debugging purposes. Primarily for trace.hh
-//
-int allFlagsVersion = 0;
+// Used to check the freshness of cached views of all flags.
FlagsMap &
allFlags()
{
+ // Ensure that the special "All" compound debug flag has been created,
+ // and avoid infinite recursion.
+ static bool done = false;
+ if (!done) {
+ done = true;
+ AllFlagsFlag::instance();
+ }
static FlagsMap flags;
return flags;
}
@@ -88,8 +92,9 @@
findFlag(const std::string &name)
{
FlagsMap::iterator i = allFlags().find(name);
- if (i == allFlags().end())
+ if (i == allFlags().end()) {
return NULL;
+ }
return i->second;
}
@@ -101,8 +106,6 @@
panic_if(!result.second, "Flag %s already defined!", name);
- ++allFlagsVersion;
-
sync();
}
@@ -127,6 +130,14 @@
i.second->sync();
}
+SimpleFlag::SimpleFlag(const char *name, const char *desc, bool is_format)
+ : Flag(name, desc), _isFormat(is_format)
+{
+ // Add non-format flags to the special "All" compound flag.
+ if (!isFormat())
+ AllFlagsFlag::instance().add(this);
+}
+
void
CompoundFlag::enable()
{
@@ -141,6 +152,26 @@
k->disable();
}
+AllFlagsFlag::AllFlagsFlag() : CompoundFlag("All",
+ "Controls all debug flags. It should not be used within C++
code.", {})
+{}
+
+void
+AllFlagsFlag::add(SimpleFlag *flag)
+{
+ ++_version;
+ _kids.push_back(flag);
+}
+
+int AllFlagsFlag::_version = 0;
+
+AllFlagsFlag &
+AllFlagsFlag::instance()
+{
+ static AllFlagsFlag flag;
+ return flag;
+}
+
bool
changeFlag(const char *s, bool value)
{
diff --git a/src/base/debug.hh b/src/base/debug.hh
index d2dfee2..f6b03ae 100644
--- a/src/base/debug.hh
+++ b/src/base/debug.hh
@@ -100,9 +100,7 @@
void sync() override { _tracing = _globalEnable && _enabled; }
public:
- SimpleFlag(const char *name, const char *desc, bool is_format=false)
- : Flag(name, desc), _isFormat(is_format)
- {}
+ SimpleFlag(const char *name, const char *desc, bool is_format=false);
void enable() override { _enabled = true; sync(); }
void disable() override { _enabled = false; sync(); }
@@ -136,6 +134,20 @@
void disable() override;
};
+class AllFlagsFlag : public CompoundFlag
+{
+ protected:
+ static int _version;
+
+ public:
+ AllFlagsFlag();
+
+ void add(SimpleFlag *flag);
+
+ static AllFlagsFlag &instance();
+ static int version() { return _version; }
+};
+
typedef std::map<std::string, Flag *> FlagsMap;
FlagsMap &allFlags();
diff --git a/src/python/pybind11/debug.cc b/src/python/pybind11/debug.cc
index 699de58..3fc3d09 100644
--- a/src/python/pybind11/debug.cc
+++ b/src/python/pybind11/debug.cc
@@ -56,12 +56,6 @@
namespace gem5
{
-GEM5_DEPRECATED_NAMESPACE(Debug, debug);
-namespace debug
-{
-extern int allFlagsVersion;
-} // namespace debug
-
static void
output(const char *filename)
{
@@ -87,7 +81,8 @@
py::module_ m_debug = m_native.def_submodule("debug");
m_debug
- .def("getAllFlagsVersion", []() { return debug::allFlagsVersion; })
+ .def("getAllFlagsVersion",
+ []() { return debug::AllFlagsFlag::version(); })
.def("allFlags", &debug::allFlags,
py::return_value_policy::reference)
.def("schedBreak", &schedBreak)
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48370
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: I8430e0fe9022846aade028fb46c80777169a2007
Gerrit-Change-Number: 48370
Gerrit-PatchSet: 12
Gerrit-Owner: Gabe Black <[email protected]>
Gerrit-Reviewer: Andreas Sandberg <[email protected]>
Gerrit-Reviewer: Daniel Carvalho <[email protected]>
Gerrit-Reviewer: Gabe Black <[email protected]>
Gerrit-Reviewer: Jason Lowe-Power <[email protected]>
Gerrit-Reviewer: Nathanael Premillieu <[email protected]>
Gerrit-Reviewer: kokoro <[email protected]>
Gerrit-CC: Jason Lowe-Power <[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
