Hi guys, @Mitch: Thanks for the detail. My argument is that introduction of the IsQuiesce instruction property in changeset 10341 now requires that if an IsQuiesce instruction is executed (i.e. a quiesce process is started), then a following EndQuiesceEvent *must* get scheduled and executed to wake up the CPU core.
@All: I just posted a patch that fixes the ARM use of UDelayEvents and returns the regression instructions and run time back to what it was. Please check it out: http://reviews.gem5.org/r/3077/ Joel On Fri, Aug 28, 2015 at 2:03 PM, Mitch Hayenga <[email protected] > wrote: > Re: Changeset 10341 > > We didn’t introduce the use of the function, we appended flags such that o3 > properly handled the existing use of these functions. > > I added this because x86 regressions were crashing on o3 after some > unrelated > changes. Basically, o3 requires an instruction to be flagged as > IsQuiesce() if it has the ability to suspend execution. This is because o3 > can leave instructions in various time buffers, etc in flight. If the CPU > happens to get ticked before it is properly woken up (this did happen back > when this patch was made), those in-flight instructions would be lost. So, > effectively the the IsQuiesce() works as a mini-barrier/drain, not letting > younger instructions into the out-of-order backend of o3. > > I remember that after writing/submitting this patch we saw the # of > instructions > on x86 regressions drastically drop on some regressions. This was because > important, after-quiesce, instructions were no longer being dropped > > > > On Fri, Aug 28, 2015 at 10:58 AM, Joel Hestness <[email protected]> > wrote: > > > Hi Andreas, > > I also saw the 30% instruction increase in these tests this morning. Am > > currently looking into it. > > > > On first read, it seems that the UDelayEvent (src/kern/linux/events.hh > > used in src/arch/arm/linux/system.cc:84-88) must be where the extra > > quiesceNs calls are originating since the kernel's timer/delay > > functionality appears to be unmodified (correct?). It looks like ARM > > hijacks the __udelay, __loop_udelay, and __loop_const_udelay kernel > > functions, swapping them with these UDelayEvents within the simulator. > > Since the quiesceNs function now suspends the CPU and reactivates it with > > an EndQuiesceEvent, it is likely that the increase in instructions a > result > > of spinning that these events skipped. > > > > I still feel strongly that every started quiesce event should result in > > an EndQuiesceEvent and that this patch should change. Changeset 10341 ( > > http://repo.gem5.org/gem5/rev/0b4d10f53c2d) introduced this requirement. > > UDelayEvents do not start a quiesce until after the event is complete, > and > > they directly call PseudoInst::quiesceNs, which I believe to be the > > funkiest use of the function in the codebase. I am testing a fix that > > conditionally calls quiesceNs from UDelayEvent::process() only if the > delay > > is >0. > > > > Will keep you posted, > > Joel > > > > > > > > On Fri, Aug 28, 2015 at 2:59 AM, ecastill <[email protected]> > wrote: > > > > > Hello, > > > > > > First of all, sorry for all the problems that the patch is causing. > > > The main issue that it aimed to solve is that when a Quiesce inst is > > > fetched, the fetch Stage completely blocks (the IsQuiesce flag in the > ISA > > > decoder). > > > If the quiesce wake event is not created, the cpu will hang and never > > wake > > > up. > > > > > > In the X86 arch. the kernel that we (and most of the user base) use, > > seems > > > to execute quiesce insts with a value of 0 ns, deadlocking the o3 CPU. > > > I have been using this patch in my own gem5 for several months already > > and > > > hadn't noticed any increase in the exec time nor the executed > > instructions. > > > I am surprised at the outcome with ARM. > > > > > > The only thing the patch do is to schedule a wake event, thus the > > > assumptions are that it should add a negligible overhead. > > > Do the ARM FS tests execute quiesce instructions of 0ns?, maybe the > > > semantics are a bit different and means something like sleep until > > > interrupt. > > > (I am completely blind when it comes to ARM). This could explain this > > > behaviour. > > > > > > Sorry again, > > > > > > Regards > > > > > > > > > On 28/08/15 09:27, Andreas Hansson wrote: > > > > > >> More problems in paradise. > > >> > > >> It turns out, on a full regression run, that the particular changeset > > >> causes a massive increase in the run-time (30%) and instruction count > > for > > >> build/ARM/tests/opt/long/fs/10.linux-boot/arm/linux/realview64-o3 and > > >> > > > build/ARM/tests/opt/long/fs/10.linux-boot/arm/linux/realview64-o3-checker. > > >> I suspect some interrupt/quiesce oddity, or even bug. Note that these > > are > > >> already some of our longest running regressions (3+ hours), and this > > >> change just added 1 hour to each of them. > > >> > > >> I am not sure what to suggest. The run-time increase is troublesome, > the > > >> fact that such a small change suddenly adds 30% to the instruction > count > > >> of a single-core regression is even more alarming. Joel, any ideas? > > Anyone > > >> else? > > >> > > >> Andreas > > >> > > >> On 27/08/2015 22:12, "gem5-dev on behalf of Andreas Hansson" > > >> <[email protected] on behalf of [email protected]> > wrote: > > >> > > >> Hi Joel, > > >>> > > >>> I always run util/regress with ‘all'. This will build all ISAs and > run > > >>> all > > >>> tests. You shouldn’t need anything else imho. > > >>> > > >>> Preferably make sure you have protobuf so that the tgen tests are run > > as > > >>> well. Eio I agree we should probably nuke, although that’s a separate > > >>> discussion. > > >>> > > >>> I would suggest the stats update should always be pushed at the same > > time > > >>> as the change that would otherwise break the regressions. Anytime > > someone > > >>> does a pull all regressions should pass. As simple as that. > > >>> > > >>> The challenge with distributing test binaries is licenses, and sheer > > >>> size. > > >>> I agree it would be good to have something openly available, and have > > >>> precompiled binaries in a separate repository or a tarball. Any > > >>> suggestions are welcome. Until we have something to replace the > current > > >>> ones we need to stick to what we have... > > >>> > > >>> For the FS ones you don’t have to modify anything, just set M5_PATH. > > >>> > > >>> Andreas > > >>> > > >>> > > >>> On 27/08/2015 20:10, "gem5-dev on behalf of Joel Hestness" > > >>> <[email protected] on behalf of [email protected]> wrote: > > >>> > > >>> On zizzer you should be able to run it without any issues. For > systems > > >>>>> other than zizzer, the FS regressions need the FS files from the > > >>>>> download > > >>>>> section on the wiki. As you point out, the cpu2000 tests rely on > > >>>>> binaries > > >>>>> we cannot distribute :-(. > > >>>>> > > >>>>> I see. A big part of my misunderstanding then is that I've never > had > > >>>> access > > >>>> to Zizzer, though I've had gem5 commit access since 2011. Is there a > > >>>> process in place for getting access to Zizzer and running > regressions > > >>>> there? > > >>>> > > >>>> > > >>>> > > >>>> A full regression run covers all ISAs and tests, not just quick. > This > > >>>>> is > > >>>>> around 50-60 host CPU hours. > > >>>>> > > >>>> > > >>>> Hm, I was not aware that this was the community's plan. This seems > > >>>> pretty > > >>>> unclear to me. For instance, the gem5 submission wiki page ( > > >>>> http://gem5.org/Submitting_Contributions) lacks precise information > > >>>> about > > >>>> which tests must be working and updated, and only states that quick > > >>>> tests > > >>>> must be run. It doesn't include anything about updating the > > regressions, > > >>>> which ones, or when the updates need to take place. > > >>>> > > >>>> Assuming all committers are on the same page about what constitutes > > >>>> "full > > >>>> regressions", I'll volunteer to update the page to clarify that the > > >>>> committer is responsible for updating all regressions as appropriate > > >>>> when > > >>>> a > > >>>> change is committed. Do we want to set a specific timeline for this > > >>>> (e.g. > > >>>> within the same set of changesets that change regressions, or within > > the > > >>>> next X days)? > > >>>> > > >>>> > > >>>> Can I also suggest a couple things?: > > >>>> 1) It is also currently unclear which tests are required based on > the > > >>>> daily cron regressions emails, and especially since the > do-regression > > >>>> script isn't available in gem5. At a minimum, we should update scons > > or > > >>>> do-regression on Zizzer to print all regression test names daily. > > >>>> Another > > >>>> good thing to do is to include do-regression in gem5, so all > > >>>> contributors > > >>>> can see how regressions are run. If we decide not to run full > > >>>> regressions > > >>>> daily, those that are not run (but need to be as part of a "full > > >>>> regression") should be marked with something more descriptive than > > >>>> "skipped" like the currently tracing regressions (eio, tgen). > > >>>> > > >>>> 2) ::Expanding on my prior email:: If we formalize "full > regressions" > > to > > >>>> include all tests, then we *really* need to include full, > > out-of-the-box > > >>>> runable regressions within the gem5 repo or at least using nicely > > >>>> packaged > > >>>> binaries/disks in a tar file (NOTE: even the FS files tarball > requires > > >>>> modifying configs/common/SysPaths.py so the simulator can find the > > >>>> files). > > >>>> It should be possible for both committers AND patch contributors to > > run > > >>>> the > > >>>> full regression suite (not just those committers with access to > > Zizzer). > > >>>> Otherwise, it seems like an unreasonable expectation for committers > > with > > >>>> access to Zizzer to completely manage regression test updates. > > >>>> > > >>>> Probably also makes sense to firm up and document committer/Zizzer > > >>>> access > > >>>> and regression plans while I go through as a partial guinea pig. > > >>>> > > >>>> Joel > > >>>> > > >>>> > > >>>> > > >>>> On 27/08/2015 14:34, "gem5-dev on behalf of Joel Hestness" > > >>>> > > >>>>> <[email protected] on behalf of [email protected]> > wrote: > > >>>>> > > >>>>> Hi Andreas, > > >>>>>> Yes, I agree that committers should run full regressions (i.e. > all > > >>>>>> > > >>>>> quick > > >>>>> > > >>>>>> tests that are verified daily on Zizzer, right?). I ran quick > > >>>>>> > > >>>>> regressions > > >>>>> > > >>>>>> that are configured correctly, and they pass with this patch. > > >>>>>> > > >>>>>> The problem with running all quick regressions currently is that > > >>>>>> > > >>>>> many > > >>>>> > > >>>>>> tests don't run out-of-the-box, and I haven't found instructions > on > > >>>>>> > > >>>>> how to > > >>>>> > > >>>>>> set up tests like the cpu2000 and FS tests. I made a request for > > such > > >>>>>> information in this thread, though I haven't yet seen response: > > >>>>>> https://www.mail-archive.com/[email protected]/msg16328.html > > >>>>>> > > >>>>>> I'm totally willing to run all the quick regressions and update > as > > >>>>>> appropriate, but I'd like more information on these. How does one > > set > > >>>>>> > > >>>>> up > > >>>>> > > >>>>>> FS > > >>>>>> and cpu2000 tests? Can we include Linux binaries in the tests/ > > >>>>>> > > >>>>> directory > > >>>>> > > >>>>>> so > > >>>>>> those tests can run out-of-the-box? Can we change the cpu2000 > tests > > to > > >>>>>> some > > >>>>>> other benchmarks without distribution restrictions, so they too > can > > be > > >>>>>> included in tests/? > > >>>>>> > > >>>>>> Thanks, > > >>>>>> Joel > > >>>>>> > > >>>>>> On Thu, Aug 27, 2015 at 7:19 AM, Andreas Hansson > > >>>>>> > > >>>>> <[email protected] > > >>>>> > > >>>>>> wrote: > > >>>>>> > > >>>>>> Hi Joel, > > >>>>>>> > > >>>>>>> It seems the patch from Emilio changes a few of the full system > > >>>>>>> regressions. I merely wanted to check if you simply forgot to > push > > a > > >>>>>>> stats > > >>>>>>> update? > > >>>>>>> > > >>>>>>> In general I would expect anyone pushing patches to do a full > > >>>>>>> > > >>>>>> regression > > >>>>> > > >>>>>> run (and sanity check any changes). Agreed? > > >>>>>>> > > >>>>>>> Thanks, > > >>>>>>> > > >>>>>>> Andreas > > >>>>>>> > > >>>>>>> > > >>>>>>> -- IMPORTANT NOTICE: The contents of this email and any > attachments > > >>>>>>> > > >>>>>> are > > >>>>> > > >>>>>> confidential and may also be privileged. If you are not the > intended > > >>>>>>> recipient, please notify the sender immediately and do not > disclose > > >>>>>>> > > >>>>>> the > > >>>>> > > >>>>>> contents to any other person, use it for any purpose, or store or > > >>>>>>> > > >>>>>> copy > > >>>>> > > >>>>>> the > > >>>>>>> information in any medium. Thank you. > > >>>>>>> > > >>>>>>> ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 > > 9NJ, > > >>>>>>> Registered in England & Wales, Company No: 2557590 > > >>>>>>> ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge > > CB1 > > >>>>>>> 9NJ, > > >>>>>>> Registered in England & Wales, Company No: 2548782 > > >>>>>>> _______________________________________________ > > >>>>>>> gem5-dev mailing list > > >>>>>>> [email protected] > > >>>>>>> http://m5sim.org/mailman/listinfo/gem5-dev > > >>>>>>> > > >>>>>>> > > >>>>>> > > >>>>>> -- > > >>>>>> Joel Hestness > > >>>>>> PhD Candidate, Computer Architecture > > >>>>>> Dept. of Computer Science, University of Wisconsin - Madison > > >>>>>> http://pages.cs.wisc.edu/~hestness/ > > >>>>>> _______________________________________________ > > >>>>>> gem5-dev mailing list > > >>>>>> [email protected] > > >>>>>> http://m5sim.org/mailman/listinfo/gem5-dev > > >>>>>> > > >>>>> > > >>>>> -- IMPORTANT NOTICE: The contents of this email and any attachments > > are > > >>>>> confidential and may also be privileged. If you are not the > intended > > >>>>> recipient, please notify the sender immediately and do not disclose > > the > > >>>>> contents to any other person, use it for any purpose, or store or > > copy > > >>>>> the > > >>>>> information in any medium. Thank you. > > >>>>> > > >>>>> ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 > 9NJ, > > >>>>> Registered in England & Wales, Company No: 2557590 > > >>>>> ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge > CB1 > > >>>>> 9NJ, > > >>>>> Registered in England & Wales, Company No: 2548782 > > >>>>> _______________________________________________ > > >>>>> gem5-dev mailing list > > >>>>> [email protected] > > >>>>> http://m5sim.org/mailman/listinfo/gem5-dev > > >>>>> > > >>>>> -- > > >>>> Joel Hestness > > >>>> PhD Candidate, Computer Architecture > > >>>> Dept. of Computer Science, University of Wisconsin - Madison > > >>>> http://pages.cs.wisc.edu/~hestness/ > > >>>> _______________________________________________ > > >>>> gem5-dev mailing list > > >>>> [email protected] > > >>>> http://m5sim.org/mailman/listinfo/gem5-dev > > >>>> > > >>> > > >>> -- IMPORTANT NOTICE: The contents of this email and any attachments > are > > >>> confidential and may also be privileged. If you are not the intended > > >>> recipient, please notify the sender immediately and do not disclose > the > > >>> contents to any other person, use it for any purpose, or store or > copy > > >>> the information in any medium. Thank you. > > >>> > > >>> ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, > > >>> Registered in England & Wales, Company No: 2557590 > > >>> ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 > > 9NJ, > > >>> Registered in England & Wales, Company No: 2548782 > > >>> _______________________________________________ > > >>> gem5-dev mailing list > > >>> [email protected] > > >>> http://m5sim.org/mailman/listinfo/gem5-dev > > >>> > > >> > > >> -- IMPORTANT NOTICE: The contents of this email and any attachments > are > > >> confidential and may also be privileged. If you are not the intended > > >> recipient, please notify the sender immediately and do not disclose > the > > >> contents to any other person, use it for any purpose, or store or copy > > the > > >> information in any medium. Thank you. > > >> > > >> ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, > > >> Registered in England & Wales, Company No: 2557590 > > >> ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 > > 9NJ, > > >> Registered in England & Wales, Company No: 2548782 > > >> _______________________________________________ > > >> gem5-dev mailing list > > >> [email protected] > > >> http://m5sim.org/mailman/listinfo/gem5-dev > > >> > > > > > > > > > WARNING / LEGAL TEXT: This message is intended only for the use of the > > > individual or entity to which it is addressed and may contain > > > information which is privileged, confidential, proprietary, or exempt > > > from disclosure under applicable law. If you are not the intended > > > recipient or the person responsible for delivering the message to the > > > intended recipient, you are strictly prohibited from disclosing, > > > distributing, copying, or in any way using this message. If you have > > > received this communication in error, please notify the sender and > > > destroy and delete any copies you may have received. > > > > > > http://www.bsc.es/disclaimer > > > > > > _______________________________________________ > > > gem5-dev mailing list > > > [email protected] > > > http://m5sim.org/mailman/listinfo/gem5-dev > > > > > > > > > > > > -- > > Joel Hestness > > PhD Candidate, Computer Architecture > > Dept. of Computer Science, University of Wisconsin - Madison > > http://pages.cs.wisc.edu/~hestness/ > > _______________________________________________ > > gem5-dev mailing list > > [email protected] > > http://m5sim.org/mailman/listinfo/gem5-dev > > > _______________________________________________ > gem5-dev mailing list > [email protected] > http://m5sim.org/mailman/listinfo/gem5-dev > > -- Joel Hestness PhD Candidate, Computer Architecture Dept. of Computer Science, University of Wisconsin - Madison http://pages.cs.wisc.edu/~hestness/ _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
