> On Aug. 13, 2015, 6:27 p.m., Tony Gutierrez wrote: > > This patch is simply changing the API without making use of the new > > argument. Please post a full patch showing how this will be used, so proper > > reviews can be done. > > Nilay Vaish wrote: > Let me give you an example from another patch: > http://reviews.gem5.org/r/3009. Look at the portion that changes the file: > src/mem/ruby/network/garnet/fixed-pipeline/InputUnit_d.cc. Earlier, on line > 70, we were calling curCycle() to get the current time in the wakeup() > function. Request 3009 would have converted that to clockEdge(). But since > this patch (3005) provides the current time as an argument to the wakeup() > function, we need not call clockEdge() at all. > > Tony Gutierrez wrote: > Is there a reason to do this though, other than to avoid calling > clockEdge() inside of wakeup()? If not then I don't think the API should > changed, i.e., those details should be hidden inside wakeup() so callers > don't need to do it.
This is a performance optimization. Calling ClockedObject::clockEdge() is more costly compared to calling Event::when(). There are only four places in ruby where wakeup() is called. So, not many changes to the callers. - Nilay ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3005/#review6941 ----------------------------------------------------------- On Aug. 9, 2015, 6:24 p.m., Nilay Vaish wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3005/ > ----------------------------------------------------------- > > (Updated Aug. 9, 2015, 6:24 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11032:95ddf324be85 > --------------------------- > ruby: consumer: pass on the current tick to wakeup() function > > This is to avoid calling clockEdge() sometimes. > > > Diffs > ----- > > src/mem/ruby/network/garnet/fixed-pipeline/VCallocator_d.hh 863d314f6356 > src/mem/ruby/network/garnet/fixed-pipeline/VCallocator_d.cc 863d314f6356 > src/mem/ruby/network/garnet/flexible-pipeline/NetworkInterface.hh > 863d314f6356 > src/mem/ruby/network/garnet/flexible-pipeline/NetworkInterface.cc > 863d314f6356 > src/mem/ruby/network/garnet/flexible-pipeline/NetworkLink.hh 863d314f6356 > src/mem/ruby/network/garnet/flexible-pipeline/NetworkLink.cc 863d314f6356 > src/mem/ruby/network/garnet/flexible-pipeline/Router.hh 863d314f6356 > src/mem/ruby/network/garnet/flexible-pipeline/Router.cc 863d314f6356 > src/mem/ruby/network/garnet/flexible-pipeline/VCarbiter.hh 863d314f6356 > src/mem/ruby/network/garnet/flexible-pipeline/VCarbiter.cc 863d314f6356 > src/mem/ruby/network/simple/PerfectSwitch.hh 863d314f6356 > src/mem/ruby/network/simple/PerfectSwitch.cc 863d314f6356 > src/mem/ruby/network/simple/Throttle.hh 863d314f6356 > src/mem/ruby/network/simple/Throttle.cc 863d314f6356 > src/mem/ruby/slicc_interface/AbstractController.hh 863d314f6356 > src/mem/ruby/structures/RubyMemoryControl.hh 863d314f6356 > src/mem/ruby/structures/RubyMemoryControl.cc 863d314f6356 > src/mem/slicc/symbols/StateMachine.py 863d314f6356 > src/mem/ruby/common/Consumer.hh 863d314f6356 > src/mem/ruby/network/garnet/fixed-pipeline/InputUnit_d.hh 863d314f6356 > src/mem/ruby/network/garnet/fixed-pipeline/InputUnit_d.cc 863d314f6356 > src/mem/ruby/network/garnet/fixed-pipeline/NetworkInterface_d.hh > 863d314f6356 > src/mem/ruby/network/garnet/fixed-pipeline/NetworkInterface_d.cc > 863d314f6356 > src/mem/ruby/network/garnet/fixed-pipeline/NetworkLink_d.hh 863d314f6356 > src/mem/ruby/network/garnet/fixed-pipeline/NetworkLink_d.cc 863d314f6356 > src/mem/ruby/network/garnet/fixed-pipeline/OutputUnit_d.hh 863d314f6356 > src/mem/ruby/network/garnet/fixed-pipeline/OutputUnit_d.cc 863d314f6356 > src/mem/ruby/network/garnet/fixed-pipeline/Router_d.cc 863d314f6356 > src/mem/ruby/network/garnet/fixed-pipeline/SWallocator_d.hh 863d314f6356 > src/mem/ruby/network/garnet/fixed-pipeline/SWallocator_d.cc 863d314f6356 > src/mem/ruby/network/garnet/fixed-pipeline/Switch_d.hh 863d314f6356 > src/mem/ruby/network/garnet/fixed-pipeline/Switch_d.cc 863d314f6356 > > Diff: http://reviews.gem5.org/r/3005/diff/ > > > Testing > ------- > > > Thanks, > > Nilay Vaish > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
