> On 2011-04-13 10:28:08, Brad Beckmann wrote:
> > configs/example/ruby_mem_test.py, line 97
> > <http://reviews.m5sim.org/r/611/diff/6/?file=11542#file11542line97>
> >
> >     It seems that the following three parameters should not be hardcoded, 
> > but instead should be set using command line options.  Hardcoding the 
> > atomic parameter to false still makes sense, since Ruby can handle atomic 
> > requests.  Also we should update the comment above.
> 
> Nilay Vaish wrote:
>     I did not add that line. I am not even aware what the purpose of that
>     line is. I'll dig into what the implication of setting issue_dmas to 
>     false is.
>     
>     I will update the comment.

Oops, that is my fault.  We actually want to keep issue_dmas hard coded to 
false and percent_uncacheable hard coded to zero.  Just add command line 
options for the percent_functional.  


> On 2011-04-13 10:28:08, Brad Beckmann wrote:
> > src/mem/protocol/MESI_CMP_directory-L1cache.sm, line 141
> > <http://reviews.m5sim.org/r/611/diff/6/?file=11548#file11548line141>
> >
> >     Why are you adding this function?  SLICC already generates a similar 
> > function: getPermission().
> >     
> >     Overall, I hope/think we can add functional access support without 
> > requiring any more changes to the protocol specific .sm files beyond the 
> > changeset:   8086:bf0335d98250 that I checked in a couple months ago.
> 
> Nilay Vaish wrote:
>     How would you use the function that is generated by SLICC inside the
>     sm file? I am concerned about the visibility of the function.

You can certainly use a function that is generated by SLICC inside the sm file. 
 The 'trigger' function is one such example.

However, I'm not clear why you need to do that?  Specifically, why do you need 
to explicitly set the permissions in the getCacheEntry function?  I beleive the 
controller's doTransition function already does that when a transition 
successfully completes.


> On 2011-04-13 10:28:08, Brad Beckmann wrote:
> > src/mem/ruby/system/CacheMemory.cc, line 270
> > <http://reviews.m5sim.org/r/611/diff/6/?file=11563#file11563line270>
> >
> >     Why are you commenting this line out?  If you think it is no longer 
> > needed, just remove it.  If we remove it, can we guarentee that the 
> > permission is initialized to some value?  For instance, do we know whether 
> > the "entry" constructor will allows initialize the value?
> 
> Nilay Vaish wrote:
>     I expect the state to decide what the permission should be.

However, if we don't set the permission to some later time, say at the end of a 
transition, what happens if we access the permission before then?


> On 2011-04-13 10:28:08, Brad Beckmann wrote:
> > src/mem/ruby/system/System.hh, line 132
> > <http://reviews.m5sim.org/r/611/diff/6/?file=11573#file11573line132>
> >
> >     Why are these functions static?  I'm concerned that declaring these 
> > static will unecessarily restrict using Ruby in multiple system simulation. 
> >  Also from my understanding of the code, there is no need to make them 
> > static.  Perhaps I'm missing something.
> 
> Nilay Vaish wrote:
>     My bad! I think I did that under the impression that static variables can
>     only be referenced in static functions.

Actually that brings up the point on why the variables are static?  It may 
belong in a separate patch, but I think we can remove the static declaration 
for all of those pointers.


- Brad


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


On 2011-04-13 14:29:01, Nilay Vaish wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/611/
> -----------------------------------------------------------
> 
> (Updated 2011-04-13 14:29:01)
> 
> 
> Review request for Default.
> 
> 
> Summary
> -------
> 
> Ruby: Add support for functional accesses
> This patch is meant for implementing functional access support in Ruby.
> Currently, the patch does not functional accesses for the PioPort.
> 
> 
> Diffs
> -----
> 
>   configs/example/ruby_mem_test.py 8b5f900233ee 
>   configs/ruby/MESI_CMP_directory.py 8b5f900233ee 
>   configs/ruby/Ruby.py 8b5f900233ee 
>   src/cpu/testers/memtest/memtest.cc 8b5f900233ee 
>   src/mem/packet.hh 8b5f900233ee 
>   src/mem/packet.cc 8b5f900233ee 
>   src/mem/protocol/MESI_CMP_directory-L1cache.sm 8b5f900233ee 
>   src/mem/protocol/MESI_CMP_directory-L2cache.sm 8b5f900233ee 
>   src/mem/protocol/MESI_CMP_directory-dir.sm 8b5f900233ee 
>   src/mem/protocol/RubySlicc_Types.sm 8b5f900233ee 
>   src/mem/ruby/network/Network.cc 8b5f900233ee 
>   src/mem/ruby/network/Network.py 8b5f900233ee 
>   src/mem/ruby/profiler/Profiler.cc 8b5f900233ee 
>   src/mem/ruby/profiler/Profiler.py 8b5f900233ee 
>   src/mem/ruby/recorder/Tracer.cc 8b5f900233ee 
>   src/mem/ruby/recorder/Tracer.py 8b5f900233ee 
>   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 8b5f900233ee 
>   src/mem/ruby/system/CacheMemory.hh 8b5f900233ee 
>   src/mem/ruby/system/CacheMemory.cc 8b5f900233ee 
>   src/mem/ruby/system/DirectoryMemory.hh 8b5f900233ee 
>   src/mem/ruby/system/DirectoryMemory.cc 8b5f900233ee 
>   src/mem/ruby/system/DirectoryMemory.py 8b5f900233ee 
>   src/mem/ruby/system/RubyPort.hh 8b5f900233ee 
>   src/mem/ruby/system/RubyPort.cc 8b5f900233ee 
>   src/mem/ruby/system/RubySystem.py 8b5f900233ee 
>   src/mem/ruby/system/SConscript 8b5f900233ee 
>   src/mem/ruby/system/Sequencer.cc 8b5f900233ee 
>   src/mem/ruby/system/Sequencer.py 8b5f900233ee 
>   src/mem/ruby/system/System.hh 8b5f900233ee 
>   src/mem/ruby/system/System.cc 8b5f900233ee 
>   src/mem/slicc/ast/MemberExprAST.py 8b5f900233ee 
> 
> Diff: http://reviews.m5sim.org/r/611/diff
> 
> 
> Testing
> -------
> 
> I have tested functional accesses with the ratio between functional
> and timing accesses for different ratios -- 100:0, 99:1, 90:1, 50:50,
> 10:90, 1:99. It is working in all the cases.
> 
> 
> Thanks,
> 
> Nilay
> 
>

_______________________________________________
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Reply via email to