Gabe Black has submitted this change and it was merged. (
https://gem5-review.googlesource.com/c/public/gem5/+/13194 )
Change subject: systemc: Centralize how object parents are chosen.
......................................................................
systemc: Centralize how object parents are chosen.
There's a lot of repeated code for this. Also, the sc_vector type
needs to be able to artificially inject a parent for the objects it
creates.
Change-Id: I76f9b551632cd2cd70e26741b215290b35c382e9
Reviewed-on: https://gem5-review.googlesource.com/c/13194
Reviewed-by: Gabe Black <[email protected]>
Maintainer: Gabe Black <[email protected]>
---
M src/systemc/core/event.cc
M src/systemc/core/module.cc
M src/systemc/core/module.hh
M src/systemc/core/object.cc
M src/systemc/core/object.hh
M src/systemc/core/sc_export.cc
M src/systemc/core/sc_module.cc
M src/systemc/core/sc_port.cc
8 files changed, 123 insertions(+), 62 deletions(-)
Approvals:
Gabe Black: Looks good to me, approved; Looks good to me, approved
diff --git a/src/systemc/core/event.cc b/src/systemc/core/event.cc
index 097961e..36830b3 100644
--- a/src/systemc/core/event.cc
+++ b/src/systemc/core/event.cc
@@ -49,17 +49,10 @@
_sc_event(_sc_event), _basename(_basename_cstr ? _basename_cstr : ""),
delayedNotify([this]() { this->notify(); }), _triggeredStamp(~0ULL)
{
- Module *p = currentModule();
-
if (_basename == "" && ::sc_core::sc_is_running())
_basename = ::sc_core::sc_gen_unique_name("event");
- if (p)
- parent = p->obj()->sc_obj();
- else if (scheduler.current())
- parent = scheduler.current();
- else
- parent = nullptr;
+ parent = pickParentObj();
std::string original_name = _basename;
_basename = pickUniqueName(parent, _basename);
diff --git a/src/systemc/core/module.cc b/src/systemc/core/module.cc
index afc3bf2..f342b7f 100644
--- a/src/systemc/core/module.cc
+++ b/src/systemc/core/module.cc
@@ -45,8 +45,6 @@
std::list<Module *> _modules;
Module *_new_module;
-Module *_callbackModule = nullptr;
-
} // anonymous namespace
Module::Module(const char *name) :
@@ -59,10 +57,15 @@
Module::~Module()
{
- if (_new_module == this) {
- // Aborted module construction?
+ // Aborted module construction?
+ if (_new_module == this)
_new_module = nullptr;
- }
+
+ // Attempt to pop now in case we're at the top of the stack, so that
+ // a stale pointer to us isn't left floating around for somebody to
trip
+ // on.
+ pop();
+
allModules.remove(this);
}
@@ -72,21 +75,29 @@
assert(!_obj);
_obj = this_obj;
_modules.push_back(this);
- _new_module = nullptr;
- // This is called from the constructor of this_obj, so it can't use
- // dynamic cast.
- sc_mod(static_cast<::sc_core::sc_module *>(this_obj->sc_obj()));
- allModules.emplace_back(this);
+ pushParentModule(this);
+ try {
+ _new_module = nullptr;
+ // This is called from the constructor of this_obj, so it can't use
+ // dynamic cast.
+ sc_mod(static_cast<::sc_core::sc_module *>(this_obj->sc_obj()));
+ allModules.emplace_back(this);
+ } catch (...) {
+ popParentModule();
+ throw;
+ }
}
void
Module::pop()
{
- panic_if(!_modules.size(), "Popping from empty module list.\n");
- panic_if(_modules.back() != this,
- "Popping module which isn't at the end of the module list.\n");
+ if (_modules.empty() || _modules.back() != this)
+ return;
+
panic_if(_new_module, "Pop with unfinished module.\n");
+
_modules.pop_back();
+ popParentModule();
}
void
@@ -111,11 +122,16 @@
void
Module::beforeEndOfElaboration()
{
- callbackModule(this);
- _sc_mod->before_end_of_elaboration();
- for (auto e: exports)
- e->before_end_of_elaboration();
- callbackModule(nullptr);
+ pushParentModule(this);
+ try {
+ _sc_mod->before_end_of_elaboration();
+ for (auto e: exports)
+ e->before_end_of_elaboration();
+ } catch (...) {
+ popParentModule();
+ throw;
+ }
+ popParentModule();
}
void
@@ -127,31 +143,46 @@
"did you forget to add a sc_module_name parameter to "
"your module constructor?", msg.c_str());
}
- callbackModule(this);
- _sc_mod->end_of_elaboration();
- for (auto e: exports)
- e->end_of_elaboration();
- callbackModule(nullptr);
+ pushParentModule(this);
+ try {
+ _sc_mod->end_of_elaboration();
+ for (auto e: exports)
+ e->end_of_elaboration();
+ } catch (...) {
+ popParentModule();
+ throw;
+ }
+ popParentModule();
}
void
Module::startOfSimulation()
{
- callbackModule(this);
- _sc_mod->start_of_simulation();
- for (auto e: exports)
- e->start_of_simulation();
- callbackModule(nullptr);
+ pushParentModule(this);
+ try {
+ _sc_mod->start_of_simulation();
+ for (auto e: exports)
+ e->start_of_simulation();
+ } catch (...) {
+ popParentModule();
+ throw;
+ }
+ popParentModule();
}
void
Module::endOfSimulation()
{
- callbackModule(this);
- _sc_mod->end_of_simulation();
- for (auto e: exports)
- e->end_of_simulation();
- callbackModule(nullptr);
+ pushParentModule(this);
+ try {
+ _sc_mod->end_of_simulation();
+ for (auto e: exports)
+ e->end_of_simulation();
+ } catch(...) {
+ popParentModule();
+ throw;
+ }
+ popParentModule();
}
Module *
@@ -179,9 +210,6 @@
return _new_module;
}
-void callbackModule(Module *m) { _callbackModule = m; }
-Module *callbackModule() { return _callbackModule; }
-
std::list<Module *> allModules;
} // namespace sc_gem5
diff --git a/src/systemc/core/module.hh b/src/systemc/core/module.hh
index aa72336..695f305 100644
--- a/src/systemc/core/module.hh
+++ b/src/systemc/core/module.hh
@@ -80,10 +80,15 @@
UniqueNameGen nameGen;
public:
-
Module(const char *name);
~Module();
+ static Module *
+ fromScModule(::sc_core::sc_module *mod)
+ {
+ return mod->_gem5_module;
+ }
+
void finish(Object *this_obj);
const char *name() const { return _name; }
@@ -130,8 +135,26 @@
Module *newModuleChecked();
Module *newModule();
-void callbackModule(Module *m);
-Module *callbackModule();
+static inline Module *
+pickParentModule()
+{
+ ::sc_core::sc_object *obj = pickParentObj();
+ auto mod = dynamic_cast<::sc_core::sc_module *>(obj);
+ if (!mod)
+ return nullptr;
+ return Module::fromScModule(mod);
+}
+static inline void
+pushParentModule(Module *m)
+{
+ pushParentObj(m->obj()->sc_obj());
+}
+static inline void
+popParentModule()
+{
+ assert(pickParentModule());
+ popParentObj();
+}
extern std::list<Module *> allModules;
diff --git a/src/systemc/core/object.cc b/src/systemc/core/object.cc
index ee6a088..91e3cb3 100644
--- a/src/systemc/core/object.cc
+++ b/src/systemc/core/object.cc
@@ -30,6 +30,7 @@
#include "systemc/core/object.hh"
#include <algorithm>
+#include <stack>
#include "base/logging.hh"
#include "systemc/core/event.hh"
@@ -91,9 +92,7 @@
if (_basename == "")
_basename = ::sc_core::sc_gen_unique_name("object");
- Module *p = currentModule();
- if (!p)
- p = callbackModule();
+ parent = pickParentObj();
Module *n = newModule();
if (n) {
@@ -101,15 +100,6 @@
n->finish(this);
}
- if (p) {
- // We're "within" a parent module, ie we're being created while its
- // constructor or end_of_elaboration callback is running.
- parent = p->obj()->_sc_obj;
- } else if (scheduler.current()) {
- // Our parent is the currently running process.
- parent = scheduler.current();
- }
-
std::string original_name = _basename;
_basename = sc_gem5::pickUniqueName(parent, original_name);
@@ -308,4 +298,27 @@
return it == allObjects.end() ? nullptr : *it;
}
+namespace
+{
+
+std::stack<sc_core::sc_object *> objParentStack;
+
+} // anonymous namespace
+
+sc_core::sc_object *
+pickParentObj()
+{
+ if (!objParentStack.empty())
+ return objParentStack.top();
+
+ Process *p = scheduler.current();
+ if (p)
+ return p;
+
+ return nullptr;
+}
+
+void pushParentObj(sc_core::sc_object *obj) { objParentStack.push(obj); }
+void popParentObj() { objParentStack.pop(); }
+
} // namespace sc_gem5
diff --git a/src/systemc/core/object.hh b/src/systemc/core/object.hh
index adccde5..cff8d84 100644
--- a/src/systemc/core/object.hh
+++ b/src/systemc/core/object.hh
@@ -113,6 +113,10 @@
sc_core::sc_object *findObject(
const char *name, const Objects &objects=topLevelObjects);
+sc_core::sc_object *pickParentObj();
+void pushParentObj(sc_core::sc_object *obj);
+void popParentObj();
+
} // namespace sc_gem5
#endif //__SYSTEMC_CORE_OBJECT_HH__
diff --git a/src/systemc/core/sc_export.cc b/src/systemc/core/sc_export.cc
index 0524ca8..9123a25 100644
--- a/src/systemc/core/sc_export.cc
+++ b/src/systemc/core/sc_export.cc
@@ -65,7 +65,7 @@
name(), kind());
}
- ::sc_gem5::Module *m = ::sc_gem5::currentModule();
+ auto m = sc_gem5::pickParentModule();
if (!m) {
reportError("(E122) sc_export specified outside of module",
nullptr, name(), kind());
diff --git a/src/systemc/core/sc_module.cc b/src/systemc/core/sc_module.cc
index d4ffda4..4605093 100644
--- a/src/systemc/core/sc_module.cc
+++ b/src/systemc/core/sc_module.cc
@@ -787,7 +787,7 @@
const char *
sc_gen_unique_name(const char *seed)
{
- ::sc_gem5::Module *mod = ::sc_gem5::currentModule();
+ auto mod = sc_gem5::pickParentModule();
return mod ? mod->uniqueName(seed) :
::sc_gem5::nameGen.gen(seed);
}
diff --git a/src/systemc/core/sc_port.cc b/src/systemc/core/sc_port.cc
index ddfe39f..3a31e41 100644
--- a/src/systemc/core/sc_port.cc
+++ b/src/systemc/core/sc_port.cc
@@ -67,7 +67,7 @@
name(), kind());
}
- ::sc_gem5::Module *m = ::sc_gem5::currentModule();
+ auto m = sc_gem5::pickParentModule();
if (!m) {
reportError("(E100) port specified outside of module",
nullptr, name(), kind());
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/13194
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings
Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-Change-Id: I76f9b551632cd2cd70e26741b215290b35c382e9
Gerrit-Change-Number: 13194
Gerrit-PatchSet: 5
Gerrit-Owner: Gabe Black <[email protected]>
Gerrit-Reviewer: Andreas Sandberg <[email protected]>
Gerrit-Reviewer: Gabe Black <[email protected]>
Gerrit-Reviewer: Giacomo Travaglini <[email protected]>
Gerrit-Reviewer: Jason Lowe-Power <[email protected]>
Gerrit-Reviewer: Matthias Jung <[email protected]>
Gerrit-MessageType: merged
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev
