> On 2011-06-13 09:21:22, Brad Beckmann wrote:
> > src/mem/protocol/MOESI_CMP_directory-dir.sm, line 473
> > <http://reviews.m5sim.org/r/611/diff/11/?file=12981#file12981line473>
> >
> >     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.

These lines need not be commented out. It seems that before changing the
access permission for certain states, this assert was failing. But after
the changes to access permission have been made, commenting out this line
is not required. Same holds for other asserts as well.


> On 2011-06-13 09:21:22, Brad Beckmann wrote:
> > src/mem/ruby/system/AbstractMemory.hh, line 43
> > <http://reviews.m5sim.org/r/611/diff/11/?file=12994#file12994line43>
> >
> >     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.

Done!


> On 2011-06-13 09:21:22, Brad Beckmann wrote:
> > src/mem/ruby/system/CacheMemory.cc, line 512
> > <http://reviews.m5sim.org/r/611/diff/11/?file=12999#file12999line512>
> >
> >     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?

You are right, I am removing the check.


> On 2011-06-13 09:21:22, Brad Beckmann wrote:
> > src/mem/ruby/system/DirectoryMemory.cc, line 258
> > <http://reviews.m5sim.org/r/611/diff/11/?file=13001#file13001line258>
> >
> >     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?

I am removing the check here as well.


> On 2011-06-13 09:21:22, Brad Beckmann wrote:
> > src/mem/protocol/MOESI_hammer-cache.sm, line 1137
> > <http://reviews.m5sim.org/r/611/diff/11/?file=12982#file12982line1137>
> >
> >     Same here?  Why comment this out?

I will uncomment these lines as well.


> On 2011-06-13 09:21:22, Brad Beckmann wrote:
> > configs/ruby/Ruby.py, line 148
> > <http://reviews.m5sim.org/r/611/diff/11/?file=12974#file12974line148>
> >
> >     Why are you exec the strings instead of just directly including the 
> > commands?

I will remove the exec call. I am not able to recall why I chose to make use of
exec. It could be that I did some thing wrong initially, which did not work
and then the exec call worked, so I left it that way.


- Nilay


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


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
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to