Gabe Black has uploaded this change for review. ( 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.

Change-Id: Ia43111d938e7af7140b1c17dd68135f426d0a1e9
---
M src/SConscript
1 file changed, 36 insertions(+), 76 deletions(-)



diff --git a/src/SConscript b/src/SConscript
index d39fb8d..48ceb52 100644
--- a/src/SConscript
+++ b/src/SConscript
@@ -1049,70 +1049,6 @@
     env.Depends(cc_file, depends + extra_deps)
     Source(cc_file)

-#
-# Handle debug flags
-#
-def makeDebugFlagCC(target, source, env):
-    assert(len(target) == 1 and len(source) == 1)
-
-    code = code_formatter()
-
-    # delay definition of CompoundFlags until after all the definition
-    # of all constituent SimpleFlags
-    comp_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
-{
-
-''')
-
-    for name, flag in sorted(source[0].read().items()):
-        n, compound, desc, fmt = flag
-        assert n == name
-
- # 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.
-        if not compound:
-            code('SimpleFlag& $name = *(')
-            code.indent()
-            if fmt:
-                code('new SimpleFlag("$name", "$desc", true)')
-            else:
-                code('new SimpleFlag("$name", "$desc", false)')
-            code.dedent()
-            code(');')
-        else:
-            comp_code('CompoundFlag& $name = *(')
-            comp_code.indent()
-            comp_code('new CompoundFlag("$name", "$desc", {')
-            comp_code.indent()
-            for flag in compound:
-                comp_code('&$flag,')
-            comp_code.dedent()
-            comp_code('})')
-            comp_code.dedent()
-            comp_code(');')
-
-    code.append(comp_code)
-    code()
-    code('} // namespace debug')
-    code('} // namespace gem5')
-
-    code.write(str(target[0]))
-
 def makeDebugFlagHH(target, source, env):
     assert(len(target) == 1 and len(source) == 1)

@@ -1127,27 +1063,55 @@
 #define __DEBUG_${name}_HH__

 #include "base/compiler.hh" // For namespace deprecation
+#include "base/debug.hh"

+''')
+    for flag in compound:
+        code('#include "debug/${flag}.hh"')
+    code('''
 namespace gem5
 {

 GEM5_DEPRECATED_NAMESPACE(Debug, debug);
 namespace debug
 {
+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 compound:
-        code('class CompoundFlag;')
-    code('class SimpleFlag;')
-
-    if compound:
-        code('extern CompoundFlag& $name;')
-        for flag in compound:
-            code('extern SimpleFlag& $flag;')
+        code('''
+static inline union $name
+{
+    ~$name() {}
+    CompoundFlag $name = {
+        "$name", "$desc", {
+            ${{",\\n            ".join(
+                f"(Flag *)&::gem5::debug::{flag}" for flag in compound)}}
+        }
+    };
+} $name;
+''')
     else:
-        code('extern SimpleFlag& $name;')
+        code('''
+static inline union $name
+{
+    ~$name() {}
+    SimpleFlag $name = {
+        "$name", "$desc", ${{"true" if fmt else "false"}}
+    };
+} $name;
+''')

     code('''
+} // namespace unions
+
+inline constexpr const auto& $name = ::gem5::debug::unions::$name.$name;
+
 } // namespace debug
 } // namespace gem5

@@ -1166,10 +1130,6 @@
     env.Command(hh_file, Value(flag),
                 MakeAction(makeDebugFlagHH, Transform("TRACING", 0)))

-env.Command('debug/flags.cc', Value(debug_flags),
-            MakeAction(makeDebugFlagCC, Transform("TRACING", 0)))
-Source('debug/flags.cc', add_tags='gem5 trace')
-
 # version tags
 tags = \
 env.Command('sim/tags.cc', None,

--
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: 1
Gerrit-Owner: Gabe Black <[email protected]>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list -- [email protected]
To unsubscribe send an email to [email protected]
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

Reply via email to