config: reinstate implicit parenting on parameter assignment

Last summer's big rewrite of the initialization code (in
particular cset 6efc3672733b) got rid of the implicit parenting
that used to occur when an unparented SimObject was assigned as
a parameter value to another SimObject.  The idea was that the
new adoptOrphanParams() step would catch these anyway so it was
unnecessary.

Unfortunately it turns out that adoptOrphanParams() has some
inherent instability in that the parent that does the adoption
depends on the config tree traversal order.  Even making this
order deterministic (e.g., by traversing children in
alphabetical order) can introduce unwanted and unexpected
hierarchy changes between similar configs (e.g., when adding a
switch_cpu in place of a cpu), causing problems when trying to
restore checkpoints across similar configs.  The hierarchy
created by implicit parenting is more stable and more
controllable, so this patch turns that behavior back on.

This patch also cleans up some long-standing holes regarding
parenting of SimObjects that are created in class definitions
(either in the body of the class, or as default parameters).

To avoid breaking some existing config files, this necessitated
changing the error on reparenting children to a warning.  This
change fixes another bug where attempting to print the prior
error message would fail on reparenting SimObjectVectors
because they lack a _parent attribute.  Some further issues
with SimObjectVectors were cleaned up by getting rid of the
get_parent() call (which could cause errors with some
SimObjectVectors where there was no single parent to return)
with has_parent() (since all the uses of get_parent() were just
boolean tests anyway).

Finally, since the adoptOrphanParam() step turned out to be so
problematic, we now issue a warning when it actually has to do
an adoption.  Future cleanup of config files will get rid of
current warnings.

2 files changed, 40 insertions, 25 deletions

diff --git a/src/python/m5/SimObject.py b/src/python/m5/SimObject.py
index 00c18976d..a143958f0 100644
--- a/src/python/m5/SimObject.py
+++ b/src/python/m5/SimObject.py
@@ -278,12 +278,26 @@ class MetaSimObject(type):
     def _set_param(cls, name, value, param):
         assert(param.name == name)
         try:
-            cls._values[name] = param.convert(value)
+            value = param.convert(value)
         except Exception, e:
             msg = "%s\nError setting param %s.%s to %s\n" % \
                   (e, cls.__name__, name, value)
             e.args = (msg, )
             raise
+        cls._values[name] = value
+        # if param value is a SimObject, make it a child too, so that
+        # it gets cloned properly when the class is instantiated
+        if isSimObjectOrVector(value) and not value.has_parent():
+            cls._add_cls_child(name, value)
+
+    def _add_cls_child(cls, name, child):
+        # It's a little funky to have a class as a parent, but these
+        # objects should never be instantiated (only cloned, which
+        # clears the parent pointer), and this makes it clear that the
+        # object is not an orphan and can provide better error
+        # messages.
+        child.set_parent(cls, name)
+        cls._children[name] = child
 
     def _new_port(cls, name, port):
         # each port should be uniquely assigned to one variable
@@ -334,7 +348,7 @@ class MetaSimObject(type):
 
         if isSimObjectOrSequence(value):
             # If RHS is a SimObject, it's an implicit child assignment.
-            cls._children[attr] = coerceSimObjectOrVector(value)
+            cls._add_cls_child(attr, coerceSimObjectOrVector(value))
             return
 
         # no valid assignment... raise exception
@@ -508,6 +522,14 @@ class SimObject(object):
         self._ccParams = None
         self._instantiated = False # really "cloned"
 
+        # Clone children specified at class level.  No need for a
+        # multidict here since we will be cloning everything.
+        # Do children before parameter values so that children that
+        # are also param values get cloned properly.
+        self._children = {}
+        for key,val in ancestor._children.iteritems():
+            self.add_child(key, val(_memo=memo_dict))
+
         # Inherit parameter values from class using multidict so
         # individual value settings can be overridden but we still
         # inherit late changes to non-overridden class values.
@@ -518,12 +540,6 @@ class SimObject(object):
             if val is not None:
                 self._values[key] = val(_memo=memo_dict)
 
-        # Clone children specified at class level.  No need for a
-        # multidict here since we will be cloning everything.
-        self._children = {}
-        for key,val in ancestor._children.iteritems():
-            self.add_child(key, val(_memo=memo_dict))
-
         # clone port references.  no need to use a multidict here
         # since we will be creating new references for all ports.
         self._port_refs = {}
@@ -614,6 +630,9 @@ class SimObject(object):
                 e.args = (msg, )
                 raise
             self._values[attr] = value
+            # implicitly parent unparented objects assigned as params
+            if isSimObjectOrVector(value) and not value.has_parent():
+                self.add_child(attr, value)
             return
 
         # if RHS is a SimObject, it's an implicit child assignment
@@ -647,10 +666,9 @@ class SimObject(object):
     def get_name(self):
         return self._name
 
-    # use this rather than directly accessing _parent for symmetry
-    # with SimObjectVector
-    def get_parent(self):
-        return self._parent
+    # Also implemented by SimObjectVector
+    def has_parent(self):
+        return self._parent is not None
 
     # clear out child with given name. This code is not likely to be exercised.
     # See comment in add_child.
@@ -662,10 +680,9 @@ class SimObject(object):
     # Add a new child to this object.
     def add_child(self, name, child):
         child = coerceSimObjectOrVector(child)
-        if child.get_parent():
-            raise RuntimeError, \
-                  "add_child('%s'): child '%s' already has parent '%s'" % \
-                  (name, child._name, child._parent)
+        if child.has_parent():
+            print "warning: add_child('%s'): child '%s' already has parent" % \
+                  (name, child.get_name())
         if self._children.has_key(name):
             # This code path had an undiscovered bug that would make it fail
             # at runtime. It had been here for a long time and was only
@@ -684,15 +701,17 @@ class SimObject(object):
         for key,val in self._values.iteritems():
             if not isSimObjectVector(val) and isSimObjectSequence(val):
                 # need to convert raw SimObject sequences to
-                # SimObjectVector class so we can call get_parent()
+                # SimObjectVector class so we can call has_parent()
                 val = SimObjectVector(val)
                 self._values[key] = val
-            if isSimObjectOrVector(val) and not val.get_parent():
+            if isSimObjectOrVector(val) and not val.has_parent():
+                print "warning: %s adopting orphan SimObject param '%s'" \
+                      % (self, key)
                 self.add_child(key, val)
 
     def path(self):
         if not self._parent:
-            return '(orphan)'
+            return '<orphan %s>' % self.__class__
         ppath = self._parent.path()
         if ppath == 'root':
             return self._name
diff --git a/src/python/m5/params.py b/src/python/m5/params.py
index 2c3925d99..86b4b0504 100644
--- a/src/python/m5/params.py
+++ b/src/python/m5/params.py
@@ -203,12 +203,8 @@ class SimObjectVector(VectorParamValue):
             for i,v in enumerate(self):
                 v.set_parent(parent, "%s%0*d" % (name, width, i))
 
-    def get_parent(self):
-        parent_set = set(v._parent for v in self)
-        if len(parent_set) != 1:
-            raise RuntimeError, \
-                  "SimObjectVector elements have inconsistent parent value."
-        return parent_set.pop()
+    def has_parent(self):
+        return reduce(lambda x,y: x and y, [v.has_parent() for v in self])
 
     # return 'cpu0 cpu1' etc. for print_ini()
     def get_name(self):