-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2458/#review5446
-----------------------------------------------------------


There is an issue with this patch for cases when a checkpoint is being 
restored. The code keeps tripping at

Module::eventLoop()
{
...
fatal("Ran out of events without seeing exit event");
}

I think keeping this fail is good and defensive. But it does not play well with 
initial PollQueue event. The way I can see the flow is:
1. SimControl::SimControl calls config_manager->instantiate()
2. CxxConfigManager::instantiate calls forEachObject(&SimObject::init)
3. this trigger cpu to schedule a poll event
4. Now this poll event triggers Module::SCEventQueue::wakeup
5. Then this causes externalSchedulingEvent to be schedule through 
Module::notify

SC_METHOD(eventLoop) has don't_initialize, so it is intentional that eventLoop 
is not entered at the very beginning. Because there is a bunch of other setup 
calls in SimControl::run, and that is where exit event is also entered into 
event queue.

Since this poll event is scheduled way early during instantiate, it ends up 
being the only event in the queue when systemc scheduler run. Exit event has 
not made it to the queue yet because systemc scheduler has not run 
SimControl::run yet. And therefore fatal is triggered.

I don't really know the reason for the poll event, and if it must be issued at 
instantiate time.

Also I understand some of the activity done in SimObject::run, but it also 
feels like initialization and simulation events are intermixed here - which is 
a bit dangerous because this is sc_thread and it is safer to have only actual 
"simulation" related activity here.

I don't have a clean way to fix this yet, so I cannot suggest a solution yet. I 
will keep looking, but I wanted to publish my thoughts here for now.

It is also a bit confusing that this is only happening in checkpoint restore 
cases, but sequence of events here suggests it should happen every time 
regardless of checkpoint_restore.

- Cagdas Dirik


On Sept. 29, 2014, 10:45 a.m., Andreas Hansson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/2458/
> -----------------------------------------------------------
> 
> (Updated Sept. 29, 2014, 10:45 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> -------
> 
> Changeset 10455:87461da3cc35
> ---------------------------
> sim: SystemC hosting
> 
> This patch hosts gem5 onto SystemC scheduler. There's already an upstream
> review board patch that does something similar but this patch ...:
> 
>  1) is less obtrusive to the existing gem5 code organisation. It's divided
>  into the 'generic' preparatory patches (already submitted) and this patch
>  which affects no existing files
> 
>  2) does not try to exactly track the gem5 event queue with notifys into
>  SystemC and so doesn't requive the event queue to be modified for
>  anything other than 'out of event queue' scheduling events
> 
>  3) supports debug logging with SC_REPORT
> 
> The patch consists of the files:
>     util/systemc/
>         sc_gem5_control.{cc,hh} -- top level objects to use to
>                                    instantiate gem5 Systems within
>                                    larger SystemC test harnesses as
>                                    sc_module objects
>         sc_logger.{cc,hh}       -- logging support
>         sc_module.{cc,hh}       -- a separated event loop specific to
>                                    SystemC
>         stats.{cc,hh}           -- example Stats handling for the sample
>                                    top level
>         main.{cc,hh}            -- a sample top level
> 
> On the downside this patch is only currently functional with C++
> configuration at the top level.
> 
> The above sc_... files are indended to be compiled alongside gem5 (as a
> library, see main.cc for a command line and util/systemc/README for
> more details.)
> 
> The top-level system instantiation in sc_gem5_control.{cc,hh} provides
> two classes: Gem5Control and Gem5System
> 
> Gem5Control is a simulation control class (from which a singleton
> object should be created) derived from Gem5SystemC::Module which
> carries the top level simulation control interface for gem5.  This
> includes hosting a system-building configuration file and
> instantiating the Root object from that file.
> 
> Gem5System is a base class for instantiating renamed gem5 Systems
> from the config file hosted by the Gem5Control object.  In use, a
> SystemC module class should be made which represents the desired,
> instantiable gem5 System.  That class's instances should create
> a Gem5System during their construction, set the parameters of that
> system and then call instantiate to build that system.  If this
> is all carried out in the sc_core::sc_module-derived classes
> constructor, the System's external ports will become children of
> that module and can then be recovered by name using sc_core::
> sc_find_object.
> 
> It is intended that this interface is used with dlopen.  To that
> end, the header file sc_gem5_control.hh includes no other header
> files from gem5 (and so can be easily copied into another project).
> The classes Gem5System and Gem5Control have all their member
> functions declared `virtual' so that those functions can be called
> through the vtable acquired by building the top level Gem5Control
> using dlsym(..., "makeGem5Control") and `makeSystem' on the
> Gem5Control.
> 
> 
> Diffs
> -----
> 
>   util/systemc/Makefile PRE-CREATION 
>   util/systemc/README PRE-CREATION 
>   util/systemc/main.cc PRE-CREATION 
>   util/systemc/sc_gem5_control.hh PRE-CREATION 
>   util/systemc/sc_gem5_control.cc PRE-CREATION 
>   util/systemc/sc_logger.hh PRE-CREATION 
>   util/systemc/sc_logger.cc PRE-CREATION 
>   util/systemc/sc_module.hh PRE-CREATION 
>   util/systemc/sc_module.cc PRE-CREATION 
>   util/systemc/stats.hh PRE-CREATION 
>   util/systemc/stats.cc PRE-CREATION 
> 
> Diff: http://reviews.gem5.org/r/2458/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andreas Hansson
> 
>

_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to