Gabe Black has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/49783 )
Change subject: scons: Use unions to prevent debug flag destruction.
......................................................................
scons: Use unions to prevent debug flag destruction.
When an object is a field in a union, it's the programmer's
resposibility to destroy it from the union's destructor. We can simply
neglect to do that and avoid having to use new to create the flags.
Also, we can define the flags as inline variables (a c++17 feature), and
then create a constexpr references to them. This lets us refer to debug
flags in constexpr objects, although we can't interact with them at, for
instance, construciton time or we'd lose our own constexpr-ness since
the actual object is not constexpr.
In c++20 we would hypothetically be able to use constexpr with new and
delete, but there may be additional restrictions that would make this
particular use impossible. Also this avoids leaking memory, which, even
though it's intentional, may confuse tools like valgrind.
Also, we need to ensure that all headers are included in some source
file so that they exist in the final executable, so that they show up in
the help, can be enabled/disabled, etc.
Change-Id: Ia43111d938e7af7140b1c17dd68135f426d0a1e9
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/49783
Maintainer: Gabe Black <[email protected]>
Reviewed-by: Jui-min Lee <[email protected]>
Tested-by: kokoro <[email protected]>
---
M build_tools/debugflaghh.py
M build_tools/debugflagcc.py
M src/SConscript
M src/arch/arm/SConscript
4 files changed, 104 insertions(+), 121 deletions(-)
Approvals:
Jui-min Lee: Looks good to me, approved
Gabe Black: Looks good to me, approved
kokoro: Regressions pass
diff --git a/build_tools/debugflagcc.py b/build_tools/debugflagcc.py
index 3dfcbad..1009c39 100644
--- a/build_tools/debugflagcc.py
+++ b/build_tools/debugflagcc.py
@@ -35,89 +35,16 @@
# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-import collections
-import sys
+import argparse
from code_formatter import code_formatter
-def usage():
- print(f"Usage: {sys.argv[0]} CC [NAME DESC FMT COMPONENTS]...",
- file=sys.stderr)
- sys.exit(1)
+parser = argparse.ArgumentParser()
+parser.add_argument("cc", help="the path of the debug flag cc file")
+parser.add_argument("name", help="the name of the debug flag")
-if len(sys.argv) < 2:
- usage()
-
-cc = sys.argv[1]
-
-
-FlagInfo = collections.namedtuple('FlagInfo',
- ['name', 'desc', 'fmt', 'components'])
-
-flags = []
-
-pos = 2
-# Extract groups of arguments for each flag.
-while pos < len(sys.argv):
- if len(sys.argv) < pos + 4:
- usage()
-
- name, desc, fmt, components = sys.argv[pos:pos+4]
- pos += 4
- fmt = fmt.lower()
- if fmt == 'true':
- fmt = True
- elif fmt == 'false':
- fmt = False
- else:
- print(f'Unrecognized "FMT" value {fmt}', file=sys.stderr)
- sys.exit(1)
- components = components.split(':') if components else []
- flags.append(FlagInfo(name, desc, fmt, components))
-
+args = parser.parse_args()
code = code_formatter()
-
-# File header.
-code('''
-#include "base/compiler.hh" // For namespace deprecation
-#include "base/debug.hh"
-
-namespace gem5
-{
-
-GEM5_DEPRECATED_NAMESPACE(Debug, debug);
-namespace debug
-{
-''')
-
-# Group the flags into either simple flags or compound flags.
-simple_flags = sorted(filter(lambda f: not f.components, flags))
-compound_flags = sorted(filter(lambda f: f.components, flags))
-
-# We intentionally make flag a reference to a heap allocated object so
-# (1) It has a similar interface to a global object like before
-# (2) It does not get destructed at the end of simulation
-# The second property is desirable as global objects from different
-# translation units do not have a defined destruction order, so it'll
-# be unsafe to access debug flags in their destructor in such cases.
-for flag in simple_flags:
- name, desc, components, fmt = \
- flag.name, flag.desc, flag.components, flag.fmt
- fmt = 'true' if fmt else 'false'
- code('''
-SimpleFlag& $name = *(
- new SimpleFlag("$name", "$desc", $fmt));''')
-
-for flag in compound_flags:
- name, desc, components = flag.name, flag.desc, flag.components
- code('''
-CompoundFlag& $name = *(new CompoundFlag("$name", "$desc", {
- ${{', '.join('&' + simple for simple in components)}}
-}));''')
-
-code('''
-} // namespace debug
-} // namespace gem5''')
-
-code.write(cc)
+code('#include "debug/${{args.name}}.hh"')
+code.write(args.cc)
diff --git a/build_tools/debugflaghh.py b/build_tools/debugflaghh.py
index fc05bfc..fc86cb0 100644
--- a/build_tools/debugflaghh.py
+++ b/build_tools/debugflaghh.py
@@ -43,6 +43,7 @@
parser = argparse.ArgumentParser()
parser.add_argument("hh", help="the path of the debug flag header file")
parser.add_argument("name", help="the name of the debug flag")
+parser.add_argument("desc", help="a description of the debug flag")
parser.add_argument("fmt",
help="whether the flag is a format flag (True or False)")
parser.add_argument("components",
@@ -50,21 +51,28 @@
args = parser.parse_args()
-fmt = eval(args.fmt)
+fmt = args.fmt.lower()
+if fmt == 'true':
+ fmt = True
+elif fmt == 'false':
+ fmt = False
+else:
+ print(f'Unrecognized "FMT" value {fmt}', file=sys.stderr)
+ sys.exit(1)
components = args.components.split(':') if args.components else []
code = code_formatter()
-typename = "CompoundFlag" if components else "SimpleFlag"
-component_flag_decls = ''.join(f'extern SimpleFlag &{simple};\n' for
- simple in components)
-
-code('''\
+code('''
#ifndef __DEBUG_${{args.name}}_HH__
#define __DEBUG_${{args.name}}_HH__
#include "base/compiler.hh" // For namespace deprecation
-
+#include "base/debug.hh"
+''')
+for flag in components:
+ code('#include "debug/${flag}.hh"')
+code('''
namespace gem5
{
@@ -72,10 +80,43 @@
namespace debug
{
-class SimpleFlag;
-class CompoundFlag;
-extern ${typename}& ${{args.name}};
-${component_flag_decls}
+namespace unions
+{
+''')
+
+# Use unions to prevent debug flags from being destructed. It's the
+# responsibility of the programmer to handle object destruction for members
+# of the union. We purposefully leave that destructor empty so that we can
+# use debug flags even in the destructors of other objects.
+if components:
+ code('''
+inline union ${{args.name}}
+{
+ ~${{args.name}}() {}
+ CompoundFlag ${{args.name}} = {
+ "${{args.name}}", "${{args.desc}}", {
+ ${{",\\n ".join(
+ f"(Flag *)&::gem5::debug::{flag}" for flag in components)}}
+ }
+ };
+} ${{args.name}};
+''')
+else:
+ code('''
+inline union ${{args.name}}
+{
+ ~${{args.name}}() {}
+ SimpleFlag ${{args.name}} = {
+ "${{args.name}}", "${{args.desc}}", ${{"true" if fmt else "false"}}
+ };
+} ${{args.name}};
+''')
+
+code('''
+} // namespace unions
+
+inline constexpr const auto& ${{args.name}} =
+ ::gem5::debug::unions::${{args.name}}.${{args.name}};
} // namespace debug
} // namespace gem5
diff --git a/src/SConscript b/src/SConscript
index db26415..866e86b 100644
--- a/src/SConscript
+++ b/src/SConscript
@@ -373,34 +373,36 @@
# Debug Flags
#
-DebugFlagInfo = collections.namedtuple('DebugFlag',
- ['name', 'desc', 'components', 'fmt'])
+debug_flags = set()
def DebugFlagCommon(name, flags, desc, fmt):
if name == "All":
raise AttributeError('The "All" flag name is reserved')
- debug_flags = env.get('DEBUG_FLAGS', [])
- if any(name == flag.name for flag in debug_flags):
+ if name in debug_flags:
raise AttributeError(f'Flag {name} already specified')
- flag = DebugFlagInfo(name, desc, flags, fmt)
- env.Append(DEBUG_FLAGS=[flag])
+ debug_flags.add(name)
hh_file = Dir(env['BUILDDIR']).Dir('debug').File(f'{name}.hh')
gem5py_env.Command(hh_file,
[ '${GEM5PY}', '${DEBUGFLAGHH_PY}' ],
MakeAction('"${GEM5PY}" "${DEBUGFLAGHH_PY}" "${TARGET}" "${NAME}" ' \
- '"${FMT}" "${COMPONENTS}"',
+ '"${DESC}" "${FMT}" "${COMPONENTS}"',
Transform("TRACING", 0)),
DEBUGFLAGHH_PY=build_tools.File('debugflaghh.py'),
- NAME=name, FMT=('True' if fmt else 'False'),
+ NAME=name, DESC=desc, FMT=('True' if fmt else 'False'),
COMPONENTS=':'.join(flags))
+ cc_file = Dir(env['BUILDDIR']).Dir('debug').File('%s.cc' % name)
+ gem5py_env.Command(cc_file,
+ [ "${GEM5PY}", "${DEBUGFLAGCC_PY}" ],
+
MakeAction('"${GEM5PY}" "${DEBUGFLAGCC_PY}" "${TARGET}" "${NAME}"',
+ Transform("TRACING", 0)),
+ DEBUGFLAGCC_PY=build_tools.File('debugflagcc.py'), NAME=name)
+ Source(cc_file, add_tags='gem5 trace')
def DebugFlag(name, desc=None, fmt=False):
DebugFlagCommon(name, (), desc, fmt)
-
def CompoundFlag(name, flags, desc=None):
DebugFlagCommon(name, flags, desc, False)
-
def DebugFormatFlag(name, desc=None):
DebugFlag(name, desc, True)
@@ -800,25 +802,6 @@
env.Depends(cc_file, depends + extra_deps)
Source(cc_file, add_tags='python')
-#
-# Handle debug flags
-#
-
-# Generate the files for the debug and debug-format flags
-flag_args = []
-for flag in env.get('DEBUG_FLAGS', []):
- components = ":".join(c for c in flag.components)
- arg = f'"{flag.name}" "{flag.desc}" "{bool(flag.fmt)}" ' \
- f'"{components}"'
- flag_args.append(arg)
-gem5py_env.Command('debug/flags.cc',
- [ "${GEM5PY}", "${DEBUGFLAGCC_PY}" ],
- MakeAction('"${GEM5PY}" "${DEBUGFLAGCC_PY}" "${TARGET}" ${FLAGS}',
- Transform("TRACING", 0)),
- DEBUGFLAGCC_PY=build_tools.File('debugflagcc.py'),
- FLAGS=' '.join(flag_args))
-Source('debug/flags.cc', add_tags='gem5 trace')
-
# version tags
tags = \
env.Command('sim/tags.cc', None,
diff --git a/src/arch/arm/SConscript b/src/arch/arm/SConscript
index ac004dd..fbf4a44 100644
--- a/src/arch/arm/SConscript
+++ b/src/arch/arm/SConscript
@@ -45,7 +45,7 @@
# Scons bug id: 2006 M5 Bug id: 308
Dir('isa/formats')
- GTest('aapcs64.test', 'aapcs64.test.cc')
+ GTest('aapcs64.test', 'aapcs64.test.cc', '../../base/debug.cc')
Source('decoder.cc')
Source('faults.cc')
Source('htm.cc')
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/49783
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: Ia43111d938e7af7140b1c17dd68135f426d0a1e9
Gerrit-Change-Number: 49783
Gerrit-PatchSet: 29
Gerrit-Owner: Gabe Black <[email protected]>
Gerrit-Reviewer: Andreas Sandberg <[email protected]>
Gerrit-Reviewer: Gabe Black <[email protected]>
Gerrit-Reviewer: Jui-min Lee <[email protected]>
Gerrit-Reviewer: Yu-hsin Wang <[email protected]>
Gerrit-Reviewer: kokoro <[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
