Just FYI, I'm working on pushing env['TARGET_ISA'] == 'foo' checks out of
SCons, and I almost have it working except that both arm and riscv define
VirtIOMMIO.py SimObject files which conflict with each other since they
take the same slot in m5.objects. You could reasonably blame that on
copy/paste coding, but there are times when you need to have basically the
same thing with some differences in two different ISAs. We can fix that by
renaming things to have an ISA prefix, but alternatively we could also put
things in packages so we don't have a flat hierarchy rooted in m5.objects
which engenders these sorts of conflicts.

Gabe

On Mon, Aug 16, 2021 at 7:59 AM Jason Lowe-Power <ja...@lowepower.com>
wrote:

> Hey Gabe,
>
> I completely agree with you that hierarchy in the Python code would be
> better! This is in part why we're working on the components library. The
> problem is supporting backwards compatibility.
>
> Cheers,
> Jason
>
> On Fri, Aug 13, 2021 at 5:49 PM Gabe Black <gabe.bl...@gmail.com> wrote:
>
>> Ok, it sounds like listing out the contents of the SimObject files is not
>> going to be a problem then, and I'll plan on doing that.
>>
>> *I* still like the idea of adding hierarchy (this is why python has
>> packages, c++ has namespaces, almost all modern languages have similar
>> features, etc), but fortunately that's an orthogonal issue to this and
>> isn't blocking. This just would make that easier to implement if we decide
>> to do that in the future.
>>
>> Gabe
>>
>> On Fri, Aug 13, 2021 at 10:21 AM Jason Lowe-Power <ja...@lowepower.com>
>> wrote:
>>
>>> Hey Gabe,
>>>
>>> Some responses inline below.
>>>
>>> On Thu, Aug 12, 2021 at 9:37 PM Gabe Black via gem5-dev <
>>> gem5-dev@gem5.org> wrote:
>>>
>>>> Hey folks, here are some of my thoughts on the SimObject() mechanism in
>>>> SCons, and the structure of the m5.objects package.
>>>>
>>>> Right now, when you put a SimObject('foo.py') line in a SConscript,
>>>> that eventually leads to SCons importing foo.py (with some special
>>>> machinery) and discovering through python what SimObjects, etc, are
>>>> declared in that file. Those are collected into lists, and then each
>>>> SimObject gets a params/Foo.hh, params/Bar.hh, etc.
>>>>
>>>> Then as far as m5.objects, each file declared with SimObject() ends up
>>>> as a subpackage, with the stuff inside it available as you would expect, ie
>>>> m5.objects.Foo.Bar and m5.objects.Foo.Foo. Also, all the SimObjects are
>>>> artificially put under m5.objects so that:
>>>>
>>>> from m5.objects import *
>>>>
>>>> will get everything.
>>>>
>>>>
>>>> First, having a single SimObject() which can produce an arbitrary
>>>> number of actual SimObject subclasses, enums, etc, is problematic for
>>>> SCons. SCons only does one pass, where you have to tell it all of what
>>>> files come from what files, and then tell it to build. That means that
>>>> SimObject() files *must* be read in during the build setup phase, since we
>>>> have to figure out what files they would produce. It would be much more
>>>> convenient to be able to delay that and do that as part of the actual
>>>> build. That would let us, for example, build the marshal binary as a build
>>>> product and not as part of setting up the build process itself.
>>>>
>>>> Second, something that I've brought up in the past is that having no
>>>> hierarchy in m5.objects is not ideal. That leads to SimObjects having
>>>> namespace information built into their name like "X86TLB", "ARMTLB", etc.
>>>> This makes things more verbose, and hides structure. It also increases the
>>>> likelihood of name collisions since everything is jammed into the same
>>>> space.
>>>>
>>>
>>> I actually think the more verbose names are better. I really don't like
>>> that we allow name "collisions" in different namespaces in C++ right now.
>>> That confuses me/our users and also confuses IDEs.
>>>
>>>
>>>>
>>>>
>>>> To help solve both of those problems, I'd like to propose changing
>>>> SimObject() to have an __all__ type parameter which specifies what inside
>>>> it to export, and what to export it as. It would also need to encode what
>>>> type of entity something is as well, so we know if we need to define an
>>>> enum, a SimObject, etc. Perhaps something like this:
>>>>
>>>> SimObject('foo.py', ['objects.x86.TLB', 'objects.x86.LocalApic',
>>>> 'enum.MyFavoriteEnum'])
>>>>
>>>
>>> I like this idea. It will make other things like supporting
>>> documentation and typing easier as well. However, I think we should not
>>> make significant changes to the python module hierarchy.
>>>
>>>
>>>>
>>>> That would tell SCons what targets to set up for, and also how to
>>>> structure the m5.objects package.
>>>>
>>>> import m5.objects.x86
>>>>
>>>> Foo.tlb = x86.TLB()
>>>>
>>>> class Example(SimObject):
>>>>     tlb = Param.x86.TLB(...)
>>>>
>>>>
>>>> This would be a major departure from how imports have worked. It would
>>>> mostly be mechanical when SimObjects are imported directly, but would make
>>>> "from m5.objects import *" impossible.\\
>>>>
>>>
>>> I think this is a huge downside. Essentially, all of the config files
>>> would have to be updated, and testing those is impossible in our current
>>> implementation. If we really think that deprecating `from m5.object import
>>> *` is going to provide large benefits for the community, then we are going
>>> to have to do it over *many* release cycles (at least 3, I would guess,
>>> maybe more).
>>>
>>>
>>>>
>>>> It would also probably also mean the params/ headers would be slightly
>>>> more complex, and would need to be something like:
>>>>
>>>> #include "params/x86/TLB.hh"
>>>>
>>>> instead of:
>>>>
>>>> #include "params/X86TLB.hh"
>>>>
>>>>
>>>> What do folks think?
>>>>
>>>> Gabe
>>>> _______________________________________________
>>>> gem5-dev mailing list -- gem5-dev@gem5.org
>>>> To unsubscribe send an email to gem5-dev-le...@gem5.org
>>>> %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
>>>
>>>
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

Reply via email to