-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/927/#review1744
-----------------------------------------------------------


Thanks for pushing this forward Nilay!  I will feel very good to see this 
functionality finally checked in.  I know people are concerned about the eventq 
manipulation, but what Nilay has implemented is much better than some of our 
other suggestions.  As a result of this change, we can start the process of 
removing all the files in src/mem/ruby/eventqueue.  That will be very nice.

My two biggest concerns are the global variables and not utilizing the recently 
added cache flush support to create valid data checkpoints of both the caches 
and memory.  See comments below.


src/mem/protocol/MOESI_CMP_token-L1cache.sm
<http://reviews.m5sim.org/r/927/#comment2235>

    Do not comment out this line.  Having to remove the clean data checks, 
along with the two event queues, were the two reasons why I didn't check in 
this patch a long time ago.  Now that we have cache flush support and 
functional access support in Ruby, we should be able to create Ruby checkpoints 
with valid data in both main memory and in the cache trace.  Therefore, we 
should not have to worry about cache warmup traces breaking this check.  If 
you're not aware of how those checks broke in the past, please let me know and 
I'm happy to discuss the details.



src/mem/protocol/MOESI_CMP_token-L2cache.sm
<http://reviews.m5sim.org/r/927/#comment2236>

    Same here.  Do not comment out.



src/mem/protocol/MOESI_CMP_token-dir.sm
<http://reviews.m5sim.org/r/927/#comment2237>

    Here too



src/mem/protocol/MOESI_hammer-cache.sm
<http://reviews.m5sim.org/r/927/#comment2238>

    here



src/mem/protocol/MOESI_hammer-dir.sm
<http://reviews.m5sim.org/r/927/#comment2239>

    here



src/mem/ruby/common/Global.hh
<http://reviews.m5sim.org/r/927/#comment2240>

    Why do these need to be global?  Can they be added to the gem5 eventqueue 
(not the ruby eventqueue) or ruby system object instead?  I seem to remember 
that is where they were before.  



src/mem/ruby/system/System.cc
<http://reviews.m5sim.org/r/927/#comment2241>

    After recording the cache contents, we should flush the cache contents to 
memory.  Then Ruby's memory image can be checkpointed with valid data.


- Brad


On 2011-12-05 08:09:50, Nilay Vaish wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/927/
> -----------------------------------------------------------
> 
> (Updated 2011-12-05 08:09:50)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> -------
> 
> Ruby: Resurrect Cache Warmup Capability
> This patch resurrects ruby's cache warmup capability.
> 
> 
> Diffs
> -----
> 
>   configs/ruby/MOESI_hammer.py c1ab57ea8805 
>   configs/ruby/Ruby.py c1ab57ea8805 
>   src/mem/SConscript c1ab57ea8805 
>   src/mem/protocol/MOESI_CMP_token-L1cache.sm c1ab57ea8805 
>   src/mem/protocol/MOESI_CMP_token-L2cache.sm c1ab57ea8805 
>   src/mem/protocol/MOESI_CMP_token-dir.sm c1ab57ea8805 
>   src/mem/protocol/MOESI_hammer-cache.sm c1ab57ea8805 
>   src/mem/protocol/MOESI_hammer-dir.sm c1ab57ea8805 
>   src/mem/ruby/buffers/MessageBuffer.cc c1ab57ea8805 
>   src/mem/ruby/common/Global.hh c1ab57ea8805 
>   src/mem/ruby/common/Global.cc c1ab57ea8805 
>   src/mem/ruby/eventqueue/RubyEventQueue.hh c1ab57ea8805 
>   src/mem/ruby/eventqueue/RubyEventQueue.cc c1ab57ea8805 
>   src/mem/ruby/network/Network.hh c1ab57ea8805 
>   src/mem/ruby/network/Network.cc c1ab57ea8805 
>   src/mem/ruby/network/Topology.cc c1ab57ea8805 
>   src/mem/ruby/network/garnet/fixed-pipeline/GarnetNetwork_d.cc c1ab57ea8805 
>   src/mem/ruby/network/garnet/fixed-pipeline/NetworkInterface_d.cc 
> c1ab57ea8805 
>   src/mem/ruby/network/garnet/fixed-pipeline/OutVcState_d.cc c1ab57ea8805 
>   src/mem/ruby/network/garnet/fixed-pipeline/RoutingUnit_d.cc c1ab57ea8805 
>   src/mem/ruby/network/garnet/flexible-pipeline/GarnetNetwork.cc c1ab57ea8805 
>   src/mem/ruby/network/garnet/flexible-pipeline/NetworkInterface.cc 
> c1ab57ea8805 
>   src/mem/ruby/network/garnet/flexible-pipeline/Router.cc c1ab57ea8805 
>   src/mem/ruby/network/simple/PerfectSwitch.cc c1ab57ea8805 
>   src/mem/ruby/network/simple/SimpleNetwork.cc c1ab57ea8805 
>   src/mem/ruby/network/simple/Switch.cc c1ab57ea8805 
>   src/mem/ruby/network/simple/Throttle.cc c1ab57ea8805 
>   src/mem/ruby/recorder/CacheRecorder.hh c1ab57ea8805 
>   src/mem/ruby/recorder/CacheRecorder.cc c1ab57ea8805 
>   src/mem/ruby/recorder/SConscript c1ab57ea8805 
>   src/mem/ruby/recorder/TraceRecord.hh c1ab57ea8805 
>   src/mem/ruby/recorder/TraceRecord.cc c1ab57ea8805 
>   src/mem/ruby/recorder/Tracer.hh c1ab57ea8805 
>   src/mem/ruby/recorder/Tracer.cc c1ab57ea8805 
>   src/mem/ruby/recorder/Tracer.py c1ab57ea8805 
>   src/mem/ruby/slicc_interface/AbstractController.hh c1ab57ea8805 
>   src/mem/ruby/slicc_interface/AbstractController.cc c1ab57ea8805 
>   src/mem/ruby/system/Cache.py c1ab57ea8805 
>   src/mem/ruby/system/CacheMemory.hh c1ab57ea8805 
>   src/mem/ruby/system/CacheMemory.cc c1ab57ea8805 
>   src/mem/ruby/system/DMASequencer.hh c1ab57ea8805 
>   src/mem/ruby/system/MemoryControl.hh c1ab57ea8805 
>   src/mem/ruby/system/MemoryControl.cc c1ab57ea8805 
>   src/mem/ruby/system/RubyPort.hh c1ab57ea8805 
>   src/mem/ruby/system/RubyPort.cc c1ab57ea8805 
>   src/mem/ruby/system/SConscript c1ab57ea8805 
>   src/mem/ruby/system/Sequencer.hh c1ab57ea8805 
>   src/mem/ruby/system/Sequencer.cc c1ab57ea8805 
>   src/mem/ruby/system/System.hh c1ab57ea8805 
>   src/mem/ruby/system/System.cc c1ab57ea8805 
>   src/mem/slicc/symbols/StateMachine.py c1ab57ea8805 
>   src/sim/eventq.hh c1ab57ea8805 
> 
> Diff: http://reviews.m5sim.org/r/927/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Nilay
> 
>

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

Reply via email to