Hi Steve, Just to chime in here. There are definitely objects that are clocked and not ticked. Thus, as Andrew already suggested, the most sensible thing to do is derive TickedObject from ClockedObject.
Andreas On 01/07/2014 18:44, "Steve Reinhardt via gem5-dev" <[email protected]> wrote: >Yes, ClockedObject, sorry. I got thrown off by TickedModule. > >Your proposal sounds great to me. Thanks a lot for being flexible. > >Steve > > >On Tue, Jul 1, 2014 at 10:33 AM, Andrew Bardsley via gem5-dev < >[email protected]> wrote: > >> Only slight modification to your proposal: >> >> How about I get rid of Ticked and flatten evaluate/minorTrace into the >> classes which need it. >> I think that there aren't actually any cases where there need to be >> virtual calls to any of >> its member functions. >> >> I assume that by ClockedModule you mean ClockedObject everywhere? >> >> I'll make a TickedObject derived class from ClockedObject that's the >>same >> as the current >> TickedModule but with 'evaluate' and move that into sim/src >> >> I'll not bother with a base-classed 'minorTrace' as I believe all the >> calls have explicitly >> known types anyway. minorTrace will appear on Pipeline. >> >> Pipeline will be (as it is now) the single instance of TickedObject in >> Minor. >> >> - Andrew >> >> -----Original Message----- >> From: gem5-dev [mailto:[email protected]] On Behalf Of Steve >> Reinhardt via gem5-dev >> Sent: 01 July 2014 17:55 >> To: gem5 Developer List >> Cc: Ali Saidi >> Subject: Re: [gem5-dev] Review Request 2279: cpu: `Minor' in-order CPU >> model >> >> Give me a chance... I haven't had time to go over Andrew's responses >> carefully. >> >> I appreciate the changes you've made so far, Andrew. I'm still not >> convinced that Ticked needs to be a separate class though. What's the >> point of an interface class when the only use of the interface is in >> conjunction with a specific implementation? Also, what does >>minorReport() >> have to do with evaluate()? It's not clear why these are part of the >>same >> interface. >> >> All of the methods in TickedModule look like they would be generally >>useful >> outside of Minor. Even if existing ClockedModules don't use them, they >> could migrate to use them eventually. And certainly the TickedModule >> methods could be useful outside of Minor. If we really think that the >>few >> extra fields in TickedModule are a burden, we could just make >>TickedModule >> derive from ClockedModule (but still push it up into src/sim). >> >> One question I don't know the answer to: are there any ClockedModule >> instances that would *not* find the TickedModule features useful? I >>think >> there is an argument for making these features "standard" across all >> ClockedModule instances. >> >> So how about this proposal: >> >> - Get rid of the Ticked interface class, and either (1) integrate the >> TickedModule features into ClockedModule or (2) add TickedModule as a >> derived class in src/sim. >> >> - Create a MinorModule class that derives from ClockedModule or >> TickedModule (depending on which way we go above) that adds the virtual >> minorReport() method. >> >> - Everything in Minor that currently derives from TickedModule can >>derive >> from MinorModule and won't know the difference. >> >> Thoughts? >> >> Steve >> >> >> >> On Tue, Jul 1, 2014 at 7:01 AM, Ali Saidi via gem5-dev >><[email protected]> >> wrote: >> >> > >> > ----------------------------------------------------------- >> > This is an automatically generated e-mail. To reply, visit: >> > http://reviews.gem5.org/r/2279/#review5171 >> > ----------------------------------------------------------- >> > >> > >> > Last chance for comments? >> > >> > - Ali Saidi >> > >> > >> > On June 17, 2014, 5:03 p.m., Ali Saidi wrote: >> > > >> > > ----------------------------------------------------------- >> > > This is an automatically generated e-mail. To reply, visit: >> > > http://reviews.gem5.org/r/2279/ >> > > ----------------------------------------------------------- >> > > >> > > (Updated June 17, 2014, 5:03 p.m.) >> > > >> > > >> > > Review request for Default. >> > > >> > > >> > > Repository: gem5 >> > > >> > > >> > > Description >> > > ------- >> > > >> > > Changeset 10237:5794a56b79c4 >> > > --------------------------- >> > > cpu: `Minor' in-order CPU model >> > > >> > > This patch contains a new CPU model named `Minor'. Minor models a >>four >> > > stage in-order execution pipeline (fetch lines, decompose into >> > > macroops, decompose macroops into microops, execute). >> > > >> > > The model was developed to support the ARM ISA but should be fixable >> > > to support all the remaining gem5 ISAs. It currently also works for >> > > Alpha, and regressions are included for ARM and Alpha (including >>Linux >> > > boot). >> > > >> > > Documentation for the model can be found in >> src/doc/inside-minor.doxygen >> > and >> > > its internal operations can be visualised using the Minorview tool >> > > utils/minorview.py. >> > > >> > > Minor was designed to be fairly simple and not to engage in a lot of >> > > instruction annotation. As such, it currently has very few gathered >> > > stats and may lack other gem5 features. >> > > >> > > Minor is faster than the o3 model. Sample results: >> > > >> > > Benchmark | Stat host_seconds (s) >> > > ---------------+--------v--------v-------- >> > > (on ARM, opt) | simple | o3 | minor >> > > | timing | timing | timing >> > > ---------------+--------+--------+-------- >> > > 10.linux-boot | 169 | 1883 | 1075 >> > > 10.mcf | 117 | 967 | 491 >> > > 20.parser | 668 | 6315 | 3146 >> > > 30.eon | 542 | 3413 | 2414 >> > > 40.perlbmk | 2339 | 20905 | 11532 >> > > 50.vortex | 122 | 1094 | 588 >> > > 60.bzip2 | 2045 | 18061 | 9662 >> > > 70.twolf | 207 | 2736 | 1036 >> > > >> > > >> > > Diffs >> > > ----- >> > > >> > > build_opts/ALPHA a2bb75a474fd >> > > build_opts/ARM a2bb75a474fd >> > > configs/common/CpuConfig.py a2bb75a474fd >> > > src/base/trace.hh a2bb75a474fd >> > > src/cpu/SConscript a2bb75a474fd >> > > src/cpu/TimingExpr.py PRE-CREATION >> > > src/cpu/minor/MinorCPU.py PRE-CREATION >> > > src/cpu/minor/SConscript PRE-CREATION >> > > src/cpu/minor/SConsopts PRE-CREATION >> > > src/cpu/minor/activity.hh PRE-CREATION >> > > src/cpu/minor/activity.cc PRE-CREATION >> > > src/cpu/minor/cpu.hh PRE-CREATION >> > > src/cpu/minor/cpu.cc PRE-CREATION >> > > src/cpu/minor/decode.hh PRE-CREATION >> > > src/cpu/minor/decode.cc PRE-CREATION >> > > src/cpu/minor/dyn_inst.hh PRE-CREATION >> > > src/cpu/minor/dyn_inst.cc PRE-CREATION >> > > src/cpu/minor/exec_context.hh PRE-CREATION >> > > src/cpu/minor/execute.hh PRE-CREATION >> > > src/cpu/minor/execute.cc PRE-CREATION >> > > src/cpu/minor/fetch1.hh PRE-CREATION >> > > src/cpu/minor/fetch1.cc PRE-CREATION >> > > src/cpu/minor/fetch2.hh PRE-CREATION >> > > src/cpu/minor/fetch2.cc PRE-CREATION >> > > src/cpu/minor/func_unit.hh PRE-CREATION >> > > src/cpu/minor/func_unit.cc PRE-CREATION >> > > src/cpu/minor/lsq.hh PRE-CREATION >> > > src/cpu/minor/lsq.cc PRE-CREATION >> > > src/cpu/minor/pipe_data.hh PRE-CREATION >> > > src/cpu/minor/pipe_data.cc PRE-CREATION >> > > src/cpu/minor/pipeline.hh PRE-CREATION >> > > src/cpu/minor/pipeline.cc PRE-CREATION >> > > src/cpu/minor/scoreboard.hh PRE-CREATION >> > > src/cpu/minor/scoreboard.cc PRE-CREATION >> > > src/cpu/minor/stage.hh PRE-CREATION >> > > src/cpu/minor/stats.hh PRE-CREATION >> > > src/cpu/minor/stats.cc PRE-CREATION >> > > src/cpu/minor/ticked.hh PRE-CREATION >> > > src/cpu/minor/trace.hh PRE-CREATION >> > > src/cpu/pred/SConscript a2bb75a474fd >> > > src/cpu/static_inst.hh a2bb75a474fd >> > > src/cpu/timing_expr.hh PRE-CREATION >> > > src/cpu/timing_expr.cc PRE-CREATION >> > > src/doc/inside-minor.doxygen PRE-CREATION >> > > util/minorview.py PRE-CREATION >> > > util/minorview/__init__.py PRE-CREATION >> > > util/minorview/blobs.py PRE-CREATION >> > > util/minorview/colours.py PRE-CREATION >> > > util/minorview/minor.pic PRE-CREATION >> > > util/minorview/model.py PRE-CREATION >> > > util/minorview/parse.py PRE-CREATION >> > > util/minorview/point.py PRE-CREATION >> > > util/minorview/view.py PRE-CREATION >> > > >> > > Diff: http://reviews.gem5.org/r/2279/diff/ >> > > >> > > >> > > Testing >> > > ------- >> > > >> > > Boots Linux and runs regression tests for ALPHA and ARM. >> > > >> > > >> > > Thanks, >> > > >> > > Ali Saidi >> > > >> > > >> > >> > _______________________________________________ >> > 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 >> >> >> -- 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 >> >_______________________________________________ >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
