This is an automatically generated e-mail. To reply, visit:

This patch has all the functionality we need.  Also I really like the 
inheritance structure.  I have a few minor questions and suggestion below.  The 
only significant change/question I have is regarding the ruby system pointer.  
Right now the pointer is passed all throughout the ruby configuration process.  
Instead of doing this, can you leverage the parent.any functionality?  I know 
it can be confusing to avoid cycles in configuration, while leveraging 
parent.any, but it seems that the current system should work.  You already do 
the back pointer setting in C++ through the register pointer callbacks.


    Why are you exec the strings instead of just directly including the 


    Why are you commenting out this check?  It seems to me that we can still 
include it, but if we cannot, we should remove the line.  Don't comment it out.


    Same here?  Why comment this out?


    It seems that the function "makeFunctionalAccess" is not turning a request 
into a functional access, but instead is actually performing the functional 
access.  How about we split these functions to "doFunctionalRead" and 
"doFunctionalWrite"?  That seems more appropriate and is more consistent with 
the rubyPort functions.


    Can you turn this into a parent any call and remove the need to pass around 
the ruby system pointer to all functions?


    It is possible to have a "-1" value here?  It seems that one only calls 
this function if the getPermission function returns Read_Only or Read_Write?  
Therefore aren't you guaranteed to find the block?


    Similar thing here.  It is possible to have the block not present here?  It 
seems that one only calls this function if the getPermission function returns 
Read_Only or Read_Write?  Therefore aren't you guaranteed to find the block?

- Brad

On 2011-06-12 14:55:53, Nilay Vaish wrote:
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/611/
> -----------------------------------------------------------
> (Updated 2011-06-12 14:55:53)
> Review request for Default.
> Summary
> -------
> Ruby: Add support for functional accesses
> This patch is meant for implementing functional access support in Ruby.
> Currently only the M5Port of RubyPort supports functional accesses. The
> support for functional through the PioPort will be added as a separate
> patch. The patch has been tested for MI, MOESI directory, MOESI hammer
> and MESI directory protocols. It seems that MOESI token protocol cannot
> support functional accesses with it current implementation, there seems
> to be some problem with the L2 cache controller.
> Diffs
> -----
>   configs/example/ruby_direct_test.py ac4da9f8ea80 
>   configs/example/ruby_fs.py ac4da9f8ea80 
>   configs/example/ruby_mem_test.py ac4da9f8ea80 
>   configs/example/ruby_network_test.py ac4da9f8ea80 
>   configs/example/ruby_random_test.py ac4da9f8ea80 
>   configs/ruby/MESI_CMP_directory.py ac4da9f8ea80 
>   configs/ruby/MI_example.py ac4da9f8ea80 
>   configs/ruby/MOESI_CMP_directory.py ac4da9f8ea80 
>   configs/ruby/MOESI_CMP_token.py ac4da9f8ea80 
>   configs/ruby/MOESI_hammer.py ac4da9f8ea80 
>   configs/ruby/Ruby.py ac4da9f8ea80 
>   src/cpu/testers/memtest/memtest.hh ac4da9f8ea80 
>   src/cpu/testers/memtest/memtest.cc ac4da9f8ea80 
>   src/mem/packet.hh ac4da9f8ea80 
>   src/mem/packet.cc ac4da9f8ea80 
>   src/mem/protocol/MOESI_CMP_directory-L1cache.sm ac4da9f8ea80 
>   src/mem/protocol/MOESI_CMP_directory-L2cache.sm ac4da9f8ea80 
>   src/mem/protocol/MOESI_CMP_directory-dir.sm ac4da9f8ea80 
>   src/mem/protocol/MOESI_hammer-cache.sm ac4da9f8ea80 
>   src/mem/protocol/MOESI_hammer-dir.sm ac4da9f8ea80 
>   src/mem/ruby/network/Network.cc ac4da9f8ea80 
>   src/mem/ruby/network/Network.py ac4da9f8ea80 
>   src/mem/ruby/profiler/Profiler.cc ac4da9f8ea80 
>   src/mem/ruby/profiler/Profiler.py ac4da9f8ea80 
>   src/mem/ruby/recorder/Tracer.cc ac4da9f8ea80 
>   src/mem/ruby/recorder/Tracer.py ac4da9f8ea80 
>   src/mem/ruby/slicc_interface/AbstractController.hh ac4da9f8ea80 
>   src/mem/ruby/slicc_interface/AbstractController.cc PRE-CREATION 
>   src/mem/ruby/slicc_interface/Controller.py ac4da9f8ea80 
>   src/mem/ruby/slicc_interface/SConscript ac4da9f8ea80 
>   src/mem/ruby/system/AbstractMemory.hh PRE-CREATION 
>   src/mem/ruby/system/AbstractMemory.cc PRE-CREATION 
>   src/mem/ruby/system/AbstractMemory.py PRE-CREATION 
>   src/mem/ruby/system/Cache.py ac4da9f8ea80 
>   src/mem/ruby/system/CacheMemory.hh ac4da9f8ea80 
>   src/mem/ruby/system/CacheMemory.cc ac4da9f8ea80 
>   src/mem/ruby/system/DirectoryMemory.hh ac4da9f8ea80 
>   src/mem/ruby/system/DirectoryMemory.cc ac4da9f8ea80 
>   src/mem/ruby/system/DirectoryMemory.py ac4da9f8ea80 
>   src/mem/ruby/system/RubyPort.hh ac4da9f8ea80 
>   src/mem/ruby/system/RubyPort.cc ac4da9f8ea80 
>   src/mem/ruby/system/RubySystem.py ac4da9f8ea80 
>   src/mem/ruby/system/SConscript ac4da9f8ea80 
>   src/mem/ruby/system/Sequencer.py ac4da9f8ea80 
>   src/mem/ruby/system/System.hh ac4da9f8ea80 
>   src/mem/ruby/system/System.cc ac4da9f8ea80 
>   src/mem/slicc/ast/MemberExprAST.py ac4da9f8ea80 
> Diff: http://reviews.m5sim.org/r/611/diff
> Testing
> -------
> Thanks,
> Nilay

gem5-dev mailing list

Reply via email to