+1 for renaming the VirtIOMMIO to be ISA-specific. Making packages in python is a good long-term goal, but right now, I think it would cause more problems than it would solve.
Cheers, Jason On Tue, Sep 14, 2021 at 4:54 AM Gabe Black <[email protected]> wrote: > 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 <[email protected]> > 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 <[email protected]> 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 <[email protected]> >>> wrote: >>> >>>> Hey Gabe, >>>> >>>> Some responses inline below. >>>> >>>> On Thu, Aug 12, 2021 at 9:37 PM Gabe Black via gem5-dev < >>>> [email protected]> 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 -- [email protected] >>>>> To unsubscribe send an email to [email protected] >>>>> %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s >>>> >>>>
_______________________________________________ gem5-dev mailing list -- [email protected] To unsubscribe send an email to [email protected] %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
