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



src/sim/eventq.hh
<http://reviews.gem5.org/r/1928/#comment4175>

    (I'm attaching this comment here as the eventq.cc patch isn't currently 
visible)
    
    I've tried a small example passing gem5 memory request around using the 
SystemC patch.  I've hit a problem with issuing gem5 events from SystemC as 
getCurTick() is allowed to lag sc_time_stamp.
    
    Both EventQueue::insert and EventQueue::remove use setCurTick to correct 
the time difference but it's quite a common paradigm in gem5 to do:
    
    event.schedule(... getCurTick() + something ...)
    
    Where getCurTick will return the wrong value.
    
    Obviously simple cases can be solved by using the 'right' source of time 
(and I can imagine scenarios where you might actually want to cultivate a time 
difference if you were, say, to have some kind of quantum keeping between 
SystemC and gem5).  Unfortunately, when the getCurTick is deep in code you have 
no control of (my immediate problem was with a schedule in SimpleMemory called 
from sendTimingReq from SystemC) it's harder to solve.
    
    I'd suggest a few solutions:
    
    1) Make the two times *always* agree.  Use sc_time_stamp().value() for 
getCurTick.
    
    2) Provide a 'catchUp' member function that can be called before any 
potentially curTick-accessing event.
    
    In both cases I'd suggest that the current setCurTicks in the SystemC 
modified EventQueue use an internal 'catchUp' and that that function should 
assert that the gem5 time is earlier than the SystemC time and that no events 
are currently scheduled between the two times.
    
    There is another problem.  EventQueue isn't actually used only for 
mainEventQueue.
    
    The CPU models use (very ugly in my opinion) EventQueues to sequence events 
based on instruction counts rather than Tick time.
    
    Look at line 140 in src/cpu/base.cc for a declaration and the function 
instDone at line 1523 in src/cpu/o3/cpu.cc for examples.
    
    I would suggest, to solve both of these problems, that when using the 
SystemC extension EventQueue should have a shallow class hierarchy.
    
    Have EventQueue in its current, non SystemC form as the base (with the 
'base is most common implementation' philosophy), make either the 
insert/remove/getCurTick interfaces virtual (or conditionally virtual if the 
virtual call scares people) or have a virtual 'synchronise/catchUp' that is 
empty in EventQueue and override those functions in a SystemCEventQueue derived 
class of which mainEventQueue is the only (rare) instance.
    
    Having that derived class would also give a home to the Gem5EventQueue 
class/instance and EventQueue::returnEvent.
    
    The event queue notify in Event::insertBefore could be implemented by an 
empty EventQueue::notifyInserted function overridden in SystemCEventQueue.


- Andrew Bardsley


On June 18, 2013, 8:43 a.m., Frédéric Konrad wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/1928/
> -----------------------------------------------------------
> 
> (Updated June 18, 2013, 8:43 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> -------
> 
> This allows Gem5 to be build with SystemC kernel when build with 'scons 
> sysc_build/*'.
> 
> NOTE: There is a small bug with SystemC 2.3.0 which is the following:
> 
>     systemc-2.3.0/include/sysc/communication/sc_interface.h:72: error: 
> "__SUNPRO_CC" is not defined
> 
> and can be fixed by replacing
> 
> private:
>       static sc_event m_never_notified;
> #if __SUNPRO_CC == 0x520
>       // Workaround for a bug in the Sun WorkShop 6 update 2 compiler.
>       // An empty virtual base class can cause the optimizer to
>       // generate wrong code.
>       char dummy;
> #endif
> };
> 
> by
> 
> private:
>       static sc_event m_never_notified;
> #if defined(__SUNPRO_CC) && (__SUNPRO_CC == 0x520)
>       // Workaround for a bug in the Sun WorkShop 6 update 2 compiler.
>       // An empty virtual base class can cause the optimizer to
>       // generate wrong code.
>       char dummy;
> #endif
> };
> 
> 
> Diffs
> -----
> 
>   SConstruct UNKNOWN 
>   src/base/pollevent.cc UNKNOWN 
>   src/python/swig/event.i UNKNOWN 
>   src/python/swig/pyevent.cc UNKNOWN 
>   src/sim/async.hh UNKNOWN 
>   src/sim/async.cc UNKNOWN 
>   src/sim/eventq.hh UNKNOWN 
>   src/sim/eventq.cc UNKNOWN 
>   src/sim/init.cc UNKNOWN 
>   src/sim/main.cc UNKNOWN 
>   src/sim/simulate.hh UNKNOWN 
>   src/sim/simulate.cc UNKNOWN 
> 
> Diff: http://reviews.gem5.org/r/1928/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Frédéric Konrad
> 
>

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

Reply via email to