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


Looking much better.
There are two main questions:
1) Do we want Ruby to prefix all trace flag names.  (I vote NO)
2) Do we want to have you insert the location into the DPRINTF statements or 
use #line directives and create DPRINTFL.  The latter is more general, but it 
could certainly be done in the future.

There are quite a number of lines that are over 80 columns, can you fix those?


src/mem/SConscript
<http://reviews.m5sim.org/r/277/#comment629>

    I still think that you should use more general names.  Try to use the names 
that the M5 cache hierarchy does.  As we merge the two, it would be better that 
way.



src/mem/protocol/MESI_CMP_directory-L1cache.sm
<http://reviews.m5sim.org/r/277/#comment621>

    Can you please make sure that you don't create lines with more than 80 
columns.



src/mem/protocol/MESI_CMP_directory-L1cache.sm
<http://reviews.m5sim.org/r/277/#comment622>

    Again



src/mem/protocol/MESI_CMP_directory-L1cache.sm
<http://reviews.m5sim.org/r/277/#comment623>

    80 columns



src/mem/protocol/MESI_CMP_directory-L1cache.sm
<http://reviews.m5sim.org/r/277/#comment624>

    80 columns



src/mem/protocol/MESI_CMP_directory-L2cache.sm
<http://reviews.m5sim.org/r/277/#comment625>

    80 columns
    



src/mem/protocol/MESI_CMP_directory-L2cache.sm
<http://reviews.m5sim.org/r/277/#comment626>

    I completely agree.



src/mem/ruby/common/NetDest.hh
<http://reviews.m5sim.org/r/277/#comment627>

    Does this compile?  Ruby isn't a trace flag.



src/mem/slicc/ast/FuncCallExprAST.py
<http://reviews.m5sim.org/r/277/#comment628>

    I think it would be better if we emitted #line statements in the output 
code instead of doing this.  That would allow us to get better compiler errors 
too.  That could be done in the future I guess.


- Nathan


On 2010-10-25 07:54:18, Nilay Vaish wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/277/
> -----------------------------------------------------------
> 
> (Updated 2010-10-25 07:54:18)
> 
> 
> Review request for Default and Ruby Reviewers.
> 
> 
> Summary
> -------
> 
> Ruby currently uses GEMS debug support with the enum character string map to 
> enable certain debug messages.  Meanwhile, M5 has debug print support that 
> works with scons. Compiling the m5.fast binary, the M5 debug statements are 
> removed, but the Ruby ones are not unless RUBY_DEBUG is not defined. This 
> patch moves Ruby to M5's debug print support.
> 
> 
> Diffs
> -----
> 
>   src/cpu/testers/rubytest/CheckTable.cc f166f8bd8818 
>   src/mem/SConscript f166f8bd8818 
>   src/mem/protocol/MESI_CMP_directory-L1cache.sm f166f8bd8818 
>   src/mem/protocol/MESI_CMP_directory-L2cache.sm f166f8bd8818 
>   src/mem/protocol/MESI_CMP_directory-dir.sm f166f8bd8818 
>   src/mem/protocol/MI_example-cache.sm f166f8bd8818 
>   src/mem/protocol/MI_example-dir.sm f166f8bd8818 
>   src/mem/protocol/MOESI_CMP_directory-L1cache.sm f166f8bd8818 
>   src/mem/protocol/MOESI_CMP_directory-L2cache.sm f166f8bd8818 
>   src/mem/protocol/MOESI_CMP_directory-dir.sm f166f8bd8818 
>   src/mem/protocol/MOESI_CMP_directory-perfectDir.sm f166f8bd8818 
>   src/mem/protocol/MOESI_CMP_token-L1cache.sm f166f8bd8818 
>   src/mem/protocol/MOESI_CMP_token-L2cache.sm f166f8bd8818 
>   src/mem/protocol/MOESI_CMP_token-dir.sm f166f8bd8818 
>   src/mem/protocol/MOESI_hammer-cache.sm f166f8bd8818 
>   src/mem/protocol/MOESI_hammer-dir.sm f166f8bd8818 
>   src/mem/ruby/SConsopts f166f8bd8818 
>   src/mem/ruby/buffers/MessageBuffer.cc f166f8bd8818 
>   src/mem/ruby/common/Debug.hh f166f8bd8818 
>   src/mem/ruby/common/Debug.cc f166f8bd8818 
>   src/mem/ruby/common/NetDest.hh f166f8bd8818 
>   src/mem/ruby/network/garnet/fixed-pipeline/NetworkInterface_d.cc 
> f166f8bd8818 
>   src/mem/ruby/network/garnet/fixed-pipeline/Switch_d.cc f166f8bd8818 
>   src/mem/ruby/network/garnet/flexible-pipeline/NetworkInterface.cc 
> f166f8bd8818 
>   src/mem/ruby/network/garnet/flexible-pipeline/Router.cc f166f8bd8818 
>   src/mem/ruby/network/simple/PerfectSwitch.cc f166f8bd8818 
>   src/mem/ruby/network/simple/Throttle.cc f166f8bd8818 
>   src/mem/ruby/network/simple/Topology.cc f166f8bd8818 
>   src/mem/ruby/storebuffer/storebuffer.cc f166f8bd8818 
>   src/mem/ruby/system/CacheMemory.cc f166f8bd8818 
>   src/mem/ruby/system/DirectoryMemory.cc f166f8bd8818 
>   src/mem/ruby/system/SConscript f166f8bd8818 
>   src/mem/ruby/system/SparseMemory.cc f166f8bd8818 
>   src/mem/ruby/tester/RaceyPseudoThread.cc f166f8bd8818 
>   src/mem/slicc/ast/FuncCallExprAST.py f166f8bd8818 
>   src/mem/slicc/symbols/StateMachine.py f166f8bd8818 
> 
> Diff: http://reviews.m5sim.org/r/277/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Nilay
> 
>

_______________________________________________
m5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/m5-dev

Reply via email to