[m5-dev] Review Request: style: move style verifiers into classes
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/642/ --- Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- style: move style verifiers into classes Diffs - util/style.py 8b5f900233ee Diff: http://reviews.m5sim.org/r/642/diff Testing --- Thanks, Nathan ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: Ruby: Add support for functional accesses
I just reviewed it. Please let me know if you have any questions. Brad -Original Message- From: m5-dev-boun...@m5sim.org [mailto:m5-dev-boun...@m5sim.org] On Behalf Of Nilay Vaish Sent: Tuesday, April 12, 2011 4:39 PM To: Default Subject: Re: [m5-dev] Review Request: Ruby: Add support for functional accesses Brad, can you take a look at the patch? I think we are now in position to implement functional accesses for the PioPort. -- Nilay On Tue, 12 Apr 2011, Nilay Vaish wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/611/ --- (Updated 2011-04-12 16:35:34.866577) Review request for Default. Summary (updated) --- Ruby: Add support for functional accesses This patch is meant for aiding discussions on implementation of functional access support in Ruby. ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: Ruby: Add support for functional accesses
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. 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. On 2011-04-13 10:28:08, Brad Beckmann wrote: src/cpu/testers/memtest/memtest.cc, line 401 http://reviews.m5sim.org/r/611/diff/6/?file=11545#file11545line401 Remove commented out line Done! 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. How would you use the function that is generated by SLICC inside the sm file? I am concerned about the visibility of the function. 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? I expect the state to decide what the permission should be. 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. My bad! I think I did that under the impression that static variables can only be referenced in static functions. On 2011-04-13 10:28:08, Brad Beckmann wrote: src/mem/ruby/system/RubyPort.cc, line 302 http://reviews.m5sim.org/r/611/diff/6/?file=11568#file11568line302 Overall, I really like the comments you added in this file. They really help clarify what is going on here. Just one minor correction: change Any copy to Any valid copy Done! On 2011-04-13 10:28:08, Brad Beckmann wrote: src/mem/ruby/system/DirectoryMemory.cc, line 246 http://reviews.m5sim.org/r/611/diff/6/?file=11565#file11565line246 Please align this statement. I am removing this line. - Nilay --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/611/#review --- On 2011-04-12 16:35:34, Nilay Vaish wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/611/ --- (Updated 2011-04-12 16:35:34) Review request for Default. Summary --- Ruby: Add support for functional accesses This patch is meant for aiding discussions on implementation of functional access support in Ruby. Diffs - configs/example/ruby_mem_test.py cb1e137ac35e configs/ruby/MESI_CMP_directory.py cb1e137ac35e configs/ruby/Ruby.py cb1e137ac35e src/cpu/testers/memtest/memtest.cc cb1e137ac35e src/mem/packet.hh cb1e137ac35e src/mem/packet.cc cb1e137ac35e src/mem/protocol/MESI_CMP_directory-L1cache.sm cb1e137ac35e src/mem/protocol/MESI_CMP_directory-L2cache.sm cb1e137ac35e src/mem/protocol/MESI_CMP_directory-dir.sm cb1e137ac35e src/mem/protocol/RubySlicc_Types.sm cb1e137ac35e src/mem/ruby/network/Network.cc cb1e137ac35e src/mem/ruby/network/Network.py cb1e137ac35e src/mem/ruby/profiler/Profiler.cc cb1e137ac35e src/mem/ruby/profiler/Profiler.py cb1e137ac35e src/mem/ruby/recorder/Tracer.cc cb1e137ac35e src/mem/ruby/recorder/Tracer.py cb1e137ac35e 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 cb1e137ac35e src/mem/ruby/system/CacheMemory.hh cb1e137ac35e src/mem/ruby/system/CacheMemory.cc cb1e137ac35e
Re: [m5-dev] Review Request: Ruby: Add support for functional accesses
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/611/ --- (Updated 2011-04-13 11:17:53.272247) Review request for Default. Summary (updated) --- 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 (updated) - 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
Re: [m5-dev] Review Request: Ruby: Add support for functional accesses
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/611/#review1113 --- Please fix these style issues, including the ones in this file I haven't explicitly pointed out. You should be using M5 style generally, but especially when your in M5 code. Also, please be sure to point this out to one of the classic memory system experts (Nate, Steve, or Ali) and get them to sign off. They might not see that this change touches outside of Ruby. src/mem/packet.hh http://reviews.m5sim.org/r/611/#comment1510 Don't add commented out code. src/mem/packet.hh http://reviews.m5sim.org/r/611/#comment1513 space after if, brace at the end of the line src/mem/packet.hh http://reviews.m5sim.org/r/611/#comment1511 space after if, brace on the end of the line not on a new one. src/mem/packet.hh http://reviews.m5sim.org/r/611/#comment1512 This should be } else { - Gabe On 2011-04-13 11:17:53, Nilay Vaish wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/611/ --- (Updated 2011-04-13 11:17:53) 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