> 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.
> 
> Brad Beckmann wrote:
>     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.
> 
> Nilay Vaish wrote:
>     I checked the generated code. It seems that permissions are being
>     set only for the cache entries and not for the directory entries.
> 
> Brad Beckmann wrote:
>     Really?  You should see a call to set_permissions inside the 
> Directory_Transitions.cc file.  For example, when I compile the MOESI_hammer 
> protocol, I see the set_permission call on line 51 in 
> Directory_Transitions.cc.
>     
>
> 
> Nilay Vaish wrote:
>     The permissions would be set for the probe filter entry and not for the
>     directory entry. The directory entry pointer is not passed around like
>     the cache entry or TBE pointer.

Doh!  Yep, that is a problem.  So what are the potential solutions:

1. Inside the setState functions for the DirectoryControllers, we also call 
set_permission.  This would require us to expose set_permissions to SLICC 
similar to how trigger is exposed to SLICC.  Certainly possilbe, but not ideal. 
 Especially because it will require directory controllers and cache controllers 
to have different functionality in their setState functions.
2. Instead of allowing an entry's state to be directly assigned in the setState 
functions, make the state variable private, thus requiring a public funciton to 
modify state.  When SLICC generates the implementation of that public function, 
have that function modify both the state and the permissions.
3. Remove the m_permission field in all entries and just rely on get_permission 
to return the current permissions for cache and directory entries.  I'm not 
sure how to do that unless we create an AbstractState class so that the state 
can be accessed by the Ruby side.  Do we want to make such a change?

If we can make it work, I would prefer the second solution.  What do you think? 
 Do you see other potential solutions?  If you agree that the second solution 
is best, do you want to take a crack at it or would you like me to?  Since it 
is my patch that is broken, I feel responsible to fix it.  However, I'm fine 
with you making the fix as well.


- 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