-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/1928/#review4458
-----------------------------------------------------------
This looks very good to me... I'm pleasantly surprised by how modest the
changes are. In addition to addressing the issues that Andrew brought up, I
suggest:
1. Adding a few high-level comments describing what's being done. I'm not a
SystemC user, but I can still make some guesses about what's happening;
nevertheless, a few brief English sentences in comments here and there would go
a long way toward making me confident that my guesses are correct.
2. Wrapping the async event stuff in a common API, so that instead of (for
example) having
#ifdef _SYSTEMC_
async_event.notify();
#else
async_event = true;
#endif
repeated all over the place, we can have a single set of functions that get
called unconditionally, with a minimum number of unique #ifdefs hidden behind
that interface.
3. Finding a way other than the 'sysc_build' name to trigger the SystemC
compilation. We already have a mechanism for compile-time configuration flags
(the "sticky" vars, like USE_FENV and TARGET_ISA). Is there a good reason not
to just add a 'USE_SYSTEMC' flag there and skip all the 'sysc_build' changes?
- Steve Reinhardt
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
