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
