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.
This commit is contained in:
Steve Reinhardt 2011-05-23 14:29:08 -07:00
parent ccbecb9e8f
commit 41fc9bbab5
2 changed files with 40 additions and 25 deletions

View file

@ -278,12 +278,26 @@ class MetaSimObject(type):
def _set_param(cls, name, value, param): def _set_param(cls, name, value, param):
assert(param.name == name) assert(param.name == name)
try: try:
cls._values[name] = param.convert(value) value = param.convert(value)
except Exception, e: except Exception, e:
msg = "%s\nError setting param %s.%s to %s\n" % \ msg = "%s\nError setting param %s.%s to %s\n" % \
(e, cls.__name__, name, value) (e, cls.__name__, name, value)
e.args = (msg, ) e.args = (msg, )
raise 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): def _new_port(cls, name, port):
# each port should be uniquely assigned to one variable # each port should be uniquely assigned to one variable
@ -334,7 +348,7 @@ class MetaSimObject(type):
if isSimObjectOrSequence(value): if isSimObjectOrSequence(value):
# If RHS is a SimObject, it's an implicit child assignment. # If RHS is a SimObject, it's an implicit child assignment.
cls._children[attr] = coerceSimObjectOrVector(value) cls._add_cls_child(attr, coerceSimObjectOrVector(value))
return return
# no valid assignment... raise exception # no valid assignment... raise exception
@ -508,6 +522,14 @@ class SimObject(object):
self._ccParams = None self._ccParams = None
self._instantiated = False # really "cloned" 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 # Inherit parameter values from class using multidict so
# individual value settings can be overridden but we still # individual value settings can be overridden but we still
# inherit late changes to non-overridden class values. # inherit late changes to non-overridden class values.
@ -518,12 +540,6 @@ class SimObject(object):
if val is not None: if val is not None:
self._values[key] = val(_memo=memo_dict) 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 # clone port references. no need to use a multidict here
# since we will be creating new references for all ports. # since we will be creating new references for all ports.
self._port_refs = {} self._port_refs = {}
@ -614,6 +630,9 @@ class SimObject(object):
e.args = (msg, ) e.args = (msg, )
raise raise
self._values[attr] = value 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 return
# if RHS is a SimObject, it's an implicit child assignment # if RHS is a SimObject, it's an implicit child assignment
@ -647,10 +666,9 @@ class SimObject(object):
def get_name(self): def get_name(self):
return self._name return self._name
# use this rather than directly accessing _parent for symmetry # Also implemented by SimObjectVector
# with SimObjectVector def has_parent(self):
def get_parent(self): return self._parent is not None
return self._parent
# clear out child with given name. This code is not likely to be exercised. # clear out child with given name. This code is not likely to be exercised.
# See comment in add_child. # See comment in add_child.
@ -662,10 +680,9 @@ class SimObject(object):
# Add a new child to this object. # Add a new child to this object.
def add_child(self, name, child): def add_child(self, name, child):
child = coerceSimObjectOrVector(child) child = coerceSimObjectOrVector(child)
if child.get_parent(): if child.has_parent():
raise RuntimeError, \ print "warning: add_child('%s'): child '%s' already has parent" % \
"add_child('%s'): child '%s' already has parent '%s'" % \ (name, child.get_name())
(name, child._name, child._parent)
if self._children.has_key(name): if self._children.has_key(name):
# This code path had an undiscovered bug that would make it fail # 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 # 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(): for key,val in self._values.iteritems():
if not isSimObjectVector(val) and isSimObjectSequence(val): if not isSimObjectVector(val) and isSimObjectSequence(val):
# need to convert raw SimObject sequences to # 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) val = SimObjectVector(val)
self._values[key] = 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) self.add_child(key, val)
def path(self): def path(self):
if not self._parent: if not self._parent:
return '(orphan)' return '<orphan %s>' % self.__class__
ppath = self._parent.path() ppath = self._parent.path()
if ppath == 'root': if ppath == 'root':
return self._name return self._name

View file

@ -203,12 +203,8 @@ class SimObjectVector(VectorParamValue):
for i,v in enumerate(self): for i,v in enumerate(self):
v.set_parent(parent, "%s%0*d" % (name, width, i)) v.set_parent(parent, "%s%0*d" % (name, width, i))
def get_parent(self): def has_parent(self):
parent_set = set(v._parent for v in self) return reduce(lambda x,y: x and y, [v.has_parent() for v in self])
if len(parent_set) != 1:
raise RuntimeError, \
"SimObjectVector elements have inconsistent parent value."
return parent_set.pop()
# return 'cpu0 cpu1' etc. for print_ini() # return 'cpu0 cpu1' etc. for print_ini()
def get_name(self): def get_name(self):