Thanks, Brad! I *hope* that the downstream work is minimal. There's only one change that I couldn't figure out a way to make backwards compatible, and it's a very minor update. BTW, if you have an idea on how to make it backwards compatible, I'm open to revising. I tried a few different things, but I couldn't find a solution.
The motivation was twofold: 1. There are two sets of changes pushed by Gabe Black that move us closer to having a single gem5 binary: the MultiISA changes and the kconfig changes. When these are merged, Ruby was the only thing holding us back from having a single gem5 binary. I think that having a single gem5 binary will be a huge win for our users. In fact, for many users they will only have to build gem5 once, or maybe not at all, if we distribute a prebuilt version! 2. Testing will be much easier/faster. I don't have data on this, yet, as the final contribution to select the protocol at runtime is missing. I'm waiting for reviews on the scons changes before I tackle the last part. That said, well over 50% (probably closer to 75% or more) of our testing is building gem5. If we can reduce the number of different targets, we're going to see a huge win, even if each build is 10% slower. I'll say that if we were to merge the changes as-is (which we shouldn't! there's still at least one large WIP), the compile time would increase. In fact, it would increase in the worst possible way. SLICC is pretty slow right now, and it runs once for every protocol during the scons startup. So, building after a change even to a single file is going to be worse with this change. Thus, I am advocating for waiting on this change (at least the part that enables multiple protocols) until after the kconfig changes have been merged. Let me know if you have any other questions! Here's the change that I couldn't figure out how to make backwards compatible: https://gem5-review.googlesource.com/c/public/gem5/+/58442 I can go into more details on that changeset, if you'd like. Here's the change that should not be merged until we have a way to easily select which protocols to build: https://gem5-review.googlesource.com/c/public/gem5/+/58443 Everything up to that change makes no difference to compile time, etc. Cheers, Jason On Thu, Mar 31, 2022 at 12:36 PM Beckmann, Brad <brad.beckm...@amd.com> wrote: > [AMD Official Use Only] > > Hi Jason, > > > > This is a huge undertaking. I’m very impressed that you got this the > work. Congratulations! > > > > Your email covers many important aspects of the change, but one item > missing is the motivation for the change. Is it primarily compilation > simplicity? Now one can build gem5 once and use that binary to run > multiple different protocols at different times? If so, can you speak to > the compilation speedup of compiling all protocols at once versus compiling > each one individually. Also does this change allow us to add more tests to > check-ins because we don’t have to spend as much time building a bunch of > individual binaries? Some quantified data could really help motivate the > downstream work involved to consume this change. > > > > Thanks! > > > > Brad > > > > > > > > > > *From:* Jason Lowe-Power via gem5-dev <gem5-dev@gem5.org> > *Sent:* Thursday, March 31, 2022 9:42 AM > *To:* gem5 Developer List <gem5-dev@gem5.org> > *Cc:* Jason Lowe-Power <ja...@lowepower.com> > *Subject:* [gem5-dev] Request for comments/reviews: Multiple Ruby > protocols in a single binary > > > > [CAUTION: External Email] > > Hi all, > > > > For as long as gem5 has been gem5, you have had to build a different gem5 > binary if you want to use a different Ruby protocol. Making it possible to > build multiple protocols has been on the roadmap for a long time (at least > as long as I've been involved with the project). > > > > I'm excited to say that we've been able to do this (finally), and we have > a set of changesets on gerrit for review/comments feedback. > > > > There are a couple of todo items before it's merged, and a couple of > user-facing changes that we could not find a way to make fully backwards > compatible. More on this below. > > > > Let me know what you think, and let me know if there are any questions! > I'm excited to see this get in for gem5-22.0. > > > > Changes: https://gem5-review.googlesource.com/q/topic:all-ruby-protocols > <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgem5-review.googlesource.com%2Fq%2Ftopic%3Aall-ruby-protocols&data=04%7C01%7Cbrad.beckmann%40amd.com%7C8d109b9ce6e1484f661508da13357d22%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637843417701279262%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=eVLWdZC6Kp0DmBoVanOw%2BASEgwXYv%2FsssJNGIZUCGqE%3D&reserved=0> > > > > *Non-backwards compatible changes:* > > Previously each SLICC protocol used the same names for the same machine > time. E.g., MI_example has an `L1Cache_Controller` and MSI has an > `L1Cache_Controller`. These names were automatically generated from the > MachineType (L1Cache) + "_Controller". Now, since we want to be able to > compile these two protocols at the same time, we need to make sure there > are no (python) name clashes. So, these have been renamed to have the > protocol name prepended onto the machine name (e.g., > `MI_example_L1Cache_Controller`). > > > > For most people using Ruby, we can provide backwards compatibility. If you > simply instantiate the `L1Cache_Controller` in python, we can provide a new > factory function that does the "right" thing. However, if you inherit from > `L1Cache_Controller` to specialize the controller, this won't work. > > > > *The user-facing change is* if you have any local ruby protocol > configuration files which use inheritance with the controllers, you will > have to update the controller classes to use the name of the protocol > prepended on the controller name. > > > > We have updated all of the configurations that are in the gem5 repo. You > will see warnings if you use the old backwards-compatible names (including > with Ruby.py). > > > > The only other user-facing change (I think, reviewers, please point out if > I'm wrong), is that in places that `buildEnv["PROTOCOL"]` was used to check > the protocol that was built, you will now need to use `getRubyProtocol` > (also available in `m5.defines`). However, we are currently supporting > backwards compatibility here, but there will be no warning when we drop > backwards compatibility (I expect this to be in a couple of releases). > > > > *To do items* > > - Correctly integrate this with scons (and the new kconfig). See > https://gem5-review.googlesource.com/c/public/gem5/+/58443 > <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgem5-review.googlesource.com%2Fc%2Fpublic%2Fgem5%2F%2B%2F58443&data=04%7C01%7Cbrad.beckmann%40amd.com%7C8d109b9ce6e1484f661508da13357d22%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637843417701279262%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=P9qJl51Qdxat2fqMiryyww52%2FMD6G0EwPK1NZMdmFEE%3D&reserved=0> > for a WIP that needs to be updated. > > - Update the website. Assuming there are no major flaws in our > implementation, we will update the website soon (before the changes are > committed). The main updates are the two user-facing changes described > above, but I expect some updates to how Ruby/SLICC works as well. > > - Add another commit that checks if the protocol is in the list of built > protocols instead of a single protocol. I'm holding off on this one until > item #1 is complete. This will also include a new command-line parameter to > set the "main" protocol for backwards compatibility. > > > > Cheers, > > Jason > > >
_______________________________________________ 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
