Re: [m5-dev] Review Request: Moving Ruby to M5's debug print support

[nathan binkert]

> Nate, the idea behind this set of changes is to do away with the debug
> support (including DEBUG_EXPR) provided by Ruby. Deciding whether given
> volume of debug output is too verbose or not will vary from situation to
> situation. Since M5 does not have a verbosity level, it is better not to
> have it in Ruby either. It should be left to users of M5 what they feel is
> verbose or not.

We control verbosity by adding flags.  If you have some flag called
Flag, you can also have a flag called FlagVerbose.

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

Re: [m5-dev] Review Request: Moving Ruby to M5's debug print support

[Nilay Vaish]

Nate, the idea behind this set of changes is to do away with the debug 
support (including DEBUG_EXPR) provided by Ruby. Deciding whether given 
volume of debug output is too verbose or not will vary from situation to 
situation. Since M5 does not have a verbosity level, it is better not to 
have it in Ruby either. It should be left to users of M5 what they feel is 
verbose or not.


Brad, the change is pretty minor (only the processing of DPRINTF() call in 
FuncCallExprAST.py has changed). As I mentioned before, whatever trace 
flag is supplied, RubySlicc is what SLICC would output. The reason to 
carry out the change is that using DPRINTF() in two different forms would 
have been confusing for the users. Note that even Steve thought that 
DPRINTF() calls are broken.


--
Nilay

On Tue, 26 Oct 2010, nathan binkert wrote:


GEMS has supported multiple filter strings ever since I can remember and one 
could have added the ability to use multiple filter strings to .sm debug 
statements in the past, but there has never been the need to.  It just seems 
redundant to explicitly specify the debug string and the .sm files are already 
quite verbose.

In my mind, we are needlessly adding a feature to the .sm files that we don't 
need and we're just further cluttering the .sm files.


My objection is based on the fact that there are several commented out
DEBUG_EXPR statements.  That usually happens when the output is too
verbose and as a result, people comment out the lines.  It would be
better to instead have another flag for a different verbosity level.
If you remove that ability, then people will again comment out lines.
Also, there is a priority parameter to DEBUG_EXPR.  Was that a
verbosity level?  Did we want to get rid of that?

I have to say that you're removing a useful feature just because it
saves 7 characters.  Just because you never used multiple flags
doesn't mean that nobody else would.  Adding more flags means that you
can put in a log more debugging statements.

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

Re: [m5-dev] Review Request: Moving Ruby to M5's debug print support

[nathan binkert]

> GEMS has supported multiple filter strings ever since I can remember and one 
> could have added the ability to use multiple filter strings to .sm debug 
> statements in the past, but there has never been the need to.  It just seems 
> redundant to explicitly specify the debug string and the .sm files are 
> already quite verbose.
>
> In my mind, we are needlessly adding a feature to the .sm files that we don't 
> need and we're just further cluttering the .sm files.

My objection is based on the fact that there are several commented out
DEBUG_EXPR statements.  That usually happens when the output is too
verbose and as a result, people comment out the lines.  It would be
better to instead have another flag for a different verbosity level.
If you remove that ability, then people will again comment out lines.
Also, there is a priority parameter to DEBUG_EXPR.  Was that a
verbosity level?  Did we want to get rid of that?

I have to say that you're removing a useful feature just because it
saves 7 characters.  Just because you never used multiple flags
doesn't mean that nobody else would.  Adding more flags means that you
can put in a log more debugging statements.

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

Re: [m5-dev] Review Request: Moving Ruby to M5's debug print support

[Beckmann, Brad]

GEMS has supported multiple filter strings ever since I can remember and one 
could have added the ability to use multiple filter strings to .sm debug 
statements in the past, but there has never been the need to.  It just seems 
redundant to explicitly specify the debug string and the .sm files are already 
quite verbose.

In my mind, we are needlessly adding a feature to the .sm files that we don't 
need and we're just further cluttering the .sm files.

Brad


-Original Message-
From: bink...@gmail.com [mailto:bink...@gmail.com] On Behalf Of nathan binkert
Sent: Tuesday, October 26, 2010 9:48 AM
To: M5 Developer List
Cc: Nilay Vaish; Beckmann, Brad; Steve Reinhardt
Subject: Re: [m5-dev] Review Request: Moving Ruby to M5's debug print support

> I would prefer not to have the .sm DPRINTF calls look exactly like the c++ 
> calls.  In particular, it seems unecessary to specify the trace flag 
> "RubySlicc" at every DPRINTF statement.  I don't think we'll ever need more 
> than one trace flag for the .sm debug statements.  We've never needed more 
> than one in the past.  Also by removing the trace flag, it will be one less 
> thing (though I understand it is relatively minor) for the protocol 
> programmer to worry about.

Famous last words.  First, I'd change RubySlicc to just Slicc (it's not like 
there's confusion there), but honestly, once you get used to having trace flags 
and start sprinkling them more liberally into the code, you'll honestly want to 
have more flags.  For example, when the protocol interacts with the cache, it 
might make more sense for that flag to be tagged with cache, or when you 
interact with the directory, etc.

I see little benefit from preventing this and I see a clear downside.


  Nate


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

Re: [m5-dev] Review Request: Moving Ruby to M5's debug print support

[Nilay Vaish]

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

(Updated 2010-10-26 10:17:01.760756)


Review request for Default and Ruby Reviewers.


Changes
---

I have condensed a couple of lines that were more than 80 characters in length. 
I had failed to add '\n' in the DPRINTF() calls. So that has been fixed as well.
Brad, though the programmer needs to specify the trace flag, SLICC compiler 
would still output RubySlicc as the trace flag.
Nate, I will go through the added trace flags later and will see which one of 
them should exist and with what names. Currently, I had simple converted the 
components that were defined in Ruby.


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 (updated)
-

  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
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: Moving Ruby to M5's debug print support

[nathan binkert]

> I would prefer not to have the .sm DPRINTF calls look exactly like the c++ 
> calls.  In particular, it seems unecessary to specify the trace flag 
> "RubySlicc" at every DPRINTF statement.  I don't think we'll ever need more 
> than one trace flag for the .sm debug statements.  We've never needed more 
> than one in the past.  Also by removing the trace flag, it will be one less 
> thing (though I understand it is relatively minor) for the protocol 
> programmer to worry about.

Famous last words.  First, I'd change RubySlicc to just Slicc (it's
not like there's confusion there), but honestly, once you get used to
having trace flags and start sprinkling them more liberally into the
code, you'll honestly want to have more flags.  For example, when the
protocol interacts with the cache, it might make more sense for that
flag to be tagged with cache, or when you interact with the directory,
etc.

I see little benefit from preventing this and I see a clear downside.


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

Re: [m5-dev] Review Request: Moving Ruby to M5's debug print support

[Brad Beckmann]

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

Ship it!


Overall, it looks good to me.  The only thing I would like to see changes is 
removing the RubySlicc flag from the .sm DPRINTF statements.

- Brad


On 2010-10-25 13:10:22, Nilay Vaish wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/277/
> ---
> 
> (Updated 2010-10-25 13:10:22)
> 
> 
> 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
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: Moving Ruby to M5's debug print support

[Brad Beckmann]

> On 2010-10-21 13:35:21, Steve Reinhardt wrote:
> > src/mem/protocol/MESI_CMP_directory-L1cache.sm, line 332
> > 
> >
> > This DPRINTF is broken (and several others like it)... it needs a trace 
> > flag as the first arg, and "%str" probably isn't the format string you want 
> > (basically it's just like printf, so you probably mean "%s").
> 
> Nilay Vaish wrote:
> The DPRINTF statements that appear in .sm files are re-written by the 
> SLICC compiler. SLICC compiler provides the trace flag. Looking at the format 
> specifier, the compiler will add a .c_str() after print_str(). But in a 
> comment above, you mentioned that c_str() is not required. May be we can drop 
> %str altogether. I need to look in to this.
> 
> Steve Reinhardt wrote:
> OK, thanks for the clarification.  I missed the SLICC compiler code 
> before but I found it now.  I have two comments on that:
> - Unless there's a real need to have a different syntax, it would be nice 
> to just write the SLICC code as a regular DPRINTF and pass DPRINTF calls 
> through unmodified.  I'd like to better understand why you think it's needed 
> (if you still think it's needed after making the c_str() change).
> - If we do end up needing a DPRINTF replacement with different syntax in 
> SLICC, let's call it something other than DPRINTF to make it clear that it is 
> different.
>
> 
> Nilay Vaish wrote:
> DPRINTF() calls will have to be modified since we want to add line number 
> of the .sm files where the call appears. I think we can maintain the syntax 
> of DPRINTF() as used in other places. But in order to combine adjacent 
> DPRINTF() statements, we would need change to the way DPRINTF() call is 
> processed. As per my understanding, currently SLICC compiler assumes that 
> only one object / variable would be printed at a time.

I would prefer not to have the .sm DPRINTF calls look exactly like the c++ 
calls.  In particular, it seems unecessary to specify the trace flag 
"RubySlicc" at every DPRINTF statement.  I don't think we'll ever need more 
than one trace flag for the .sm debug statements.  We've never needed more than 
one in the past.  Also by removing the trace flag, it will be one less thing 
(though I understand it is relatively minor) for the protocol programmer to 
worry about.


> On 2010-10-21 13:35:21, Steve Reinhardt wrote:
> > src/mem/SConscript, line 61
> > 
> >
> > Do we want to define a whole new disjoint set of trace flags just for 
> > Ruby?  For example, why have RubyCache and Cache both?

I don't think we need a whole disjoint set of trace flags and for most new 
flags, I don't think we need to explicitly identify them as Ruby flags.  
However, I can see how one would want a separate RubyCache flag since we 
currently have both a RubyCache and Cache configuration python object.  
Overall, I'm indifferent and I'm willing to let Nilay use his best judgement 
here or let others insist on one way or another.


- Brad


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


On 2010-10-25 13:10:22, Nilay Vaish wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/277/
> ---
> 
> (Updated 2010-10-25 13:10:22)
> 
> 
> 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 f166f8bd8

Re: [m5-dev] Review Request: Moving Ruby to M5's debug print support

[Nilay Vaish]

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

(Updated 2010-10-25 13:10:22.143949)


Review request for Default and Ruby Reviewers.


Changes
---

I have converted all the lines to <= 80 characters. But not all lines in the 
.sm files cannot follow this convention since SLICC does not support strings 
that are split across two different lines.


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 (updated)
-

  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
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: Moving Ruby to M5's debug print support

[Nathan Binkert]

---
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


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


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



src/mem/protocol/MESI_CMP_directory-L1cache.sm


Again



src/mem/protocol/MESI_CMP_directory-L1cache.sm


80 columns



src/mem/protocol/MESI_CMP_directory-L1cache.sm


80 columns



src/mem/protocol/MESI_CMP_directory-L2cache.sm


80 columns




src/mem/protocol/MESI_CMP_directory-L2cache.sm


I completely agree.



src/mem/ruby/common/NetDest.hh


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



src/mem/slicc/ast/FuncCallExprAST.py


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
> 

Re: [m5-dev] Review Request: Moving Ruby to M5's debug print support

[nathan binkert]

> If we just want to insert __FILE__ and __LINE__ as part of the prefix,
> we should define a new flavor of DPRINTF that does that in the same
> header where DPRINTF itself is, and use that version as-is in SLICC.
> Maybe call it DPRINTFL?

Which __FILE__ and __LINE__ are getting inserted?  Something from the
.sm file, or something from the generated code.  If it's the .sm file,
then __FILE__/__LINE__ wouldn't work unless we make slicc output #line
statements for the preprocessor (a good idea IMHO)

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

Re: [m5-dev] Review Request: Moving Ruby to M5's debug print support

[nathan binkert]

> Does that mean that format specifiers used in DPRINTF() statements are just 
> place holders? In not, what would be the format specifier for printing an 
> object of some class that supports << operator?

No.  %s passes a string in an unprocessed way, but the other format
specifiers do various ostream manipulations to mimic printf.  I tried
pretty hard to have cprintf match printf exactly.
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: Moving Ruby to M5's debug print support

[Steve Reinhardt]

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



src/mem/protocol/MESI_CMP_directory-L2cache.sm


It looks like this call goes over 80 chars on the first line, and then uses 
up a lot of unnecessary lines by taking up a whole line for each arg.  If you 
want to keep the verbose labels, you should format it like this:

DPRINTF(RubySlicc, "Address: %s, L2 Cache State: %s, Sender: %s, "
"Response Type: %s, Destination: %s",
in_msg.Address, getState(in_msg.Address), in_msg.Sender,
in_msg.Type, in_msg.Destination);

though my inclination would be to save more space by abbreviating or even 
eliminating some of the labels in the format string (since most of the fields 
will be self-evident, and they'll get pretty tiring in the trace output too, I 
expect).



- Steve


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
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: Moving Ruby to M5's debug print support

[Steve Reinhardt]

On Mon, Oct 25, 2010 at 7:57 AM, Nilay Vaish  wrote:
>
>
>> On 2010-10-24 21:45:51, Steve Reinhardt wrote:
>> > src/mem/SConscript, line 61
>> > 
>> >
>> >     Nate and I agreed that it would be nice not to prefix all of these 
>> > with "Ruby", though if other people feel this decision needs more 
>> > discussion that's fine.
>
> To me it seems that cache modeling is forked into two portions, one under the 
> cache directory and another under the ruby directory. Therefore, I would 
> prefer having separate trace flags for these two.

It seems very unlikely that anyone would be using both flavors of
memory hierarchy models at the same time, so the ability to trace one
and not the other doesn't seem valuable to me.  Also, I believe the
goal in the long run is to make Ruby "the" GEM5 memory system, at
which point all these "Ruby" labels will seem very redundant.  Not a
huge deal, but I'm curious how others feel.

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

Re: [m5-dev] Review Request: Moving Ruby to M5's debug print support

[Steve Reinhardt]

On Mon, Oct 25, 2010 at 7:54 AM, Nilay Vaish  wrote:
>
> I have removed the _to_string() function calls that were appearing in .sm 
> files. Also, I have added a comment for the changes made to the 
> FuncCallExprAST.py.

Great, thanks.  I didn't realize that the operator<< definitions
already existed for these types, which makes it even more of a win.

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

Re: [m5-dev] Review Request: Moving Ruby to M5's debug print support

[Nilay Vaish]

> On 2010-10-24 21:45:51, Steve Reinhardt wrote:
> > src/mem/SConscript, line 61
> > 
> >
> > Nate and I agreed that it would be nice not to prefix all of these with 
> > "Ruby", though if other people feel this decision needs more discussion 
> > that's fine.

To me it seems that cache modeling is forked into two portions, one under the 
cache directory and another under the ruby directory. Therefore, I would prefer 
having separate trace flags for these two.


- Nilay


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


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
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: Moving Ruby to M5's debug print support

[Nilay Vaish]

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

(Updated 2010-10-25 07:54:18.182412)


Review request for Default and Ruby Reviewers.


Changes
---

I have removed the _to_string() function calls that were appearing in .sm 
files. Also, I have added a comment for the changes made to the 
FuncCallExprAST.py.


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 (updated)
-

  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
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: Moving Ruby to M5's debug print support

[Nilay Vaish]

> On 2010-10-24 21:45:51, Steve Reinhardt wrote:
> > src/mem/ruby/storebuffer/storebuffer.cc, line 163
> > 
> >
> > In these cases (and the ones below) where this is really a fatal error, 
> > I believe the whole ERROR_OUT/DEBUG_EXPR/ASSERT(0) sequence should be 
> > replaced by a single call to panic(), which takes the same arguments as 
> > cprintf().  I think if you're dying you don't want to only print relevant 
> > info if you had traceflags enabled too.

Let's handle ERROR_OUT calls in a different set of changes. We will be going 
through Ruby's assert mechanism soon.


> On 2010-10-24 21:45:51, Steve Reinhardt wrote:
> > src/mem/slicc/ast/FuncCallExprAST.py, line 51
> > 
> >
> > What is this code doing?  It looks like it's inserting the location, 
> > but how does this work if the format string isn't changed?  Plus it would 
> > be nice to have a comment here in the SLICC code too.

The code inserts the location of the DPRINTF() statement in the .sm file in the 
statement it self. The second argument in the call to 'code()' function 
represents the location. This is placed where $0 appears in the first argument 
of the 'code()' function. 'format' represents the second argument of the 
original DPRINTF() call. It is left unmodified. str_list is used for 
concatenating the argument list following the format specifier in a DPRINTF() 
call. A DPRINTF() call may or may not contain any arguments following the 
format specifier. Consider the examples DPRINTF(RubySlicc,"Hello!") and 
DPRINTF(RubySlicc,"%s",address). These two need to be handled differently. 
Hence the check whether or not the str_list is empty.


> On 2010-10-24 21:45:51, Steve Reinhardt wrote:
> > src/mem/protocol/MI_example-dir.sm, line 170
> > 
> >
> > Can some of these FooBarBazType_to_string() functions, like the one 
> > here and several more below, be converted to (or hidden behind) operator< >  That's not strictly necessary but it would make these calls a lot less 
> > verbose.

I will make these changes and update the diff soon.


- Nilay


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


On 2010-10-24 08:59:56, Nilay Vaish wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/277/
> ---
> 
> (Updated 2010-10-24 08:59:56)
> 
> 
> 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 f166f8bd

Re: [m5-dev] Review Request: Moving Ruby to M5's debug print support

[Steve Reinhardt]

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


Thanks, this is hugely improved.  Just a few little things I noticed still; the 
panic() thing is the only one I consider really necessary to take care of 
before committing.


src/mem/SConscript


Nate and I agreed that it would be nice not to prefix all of these with 
"Ruby", though if other people feel this decision needs more discussion that's 
fine.



src/mem/protocol/MI_example-dir.sm


Can some of these FooBarBazType_to_string() functions, like the one here 
and several more below, be converted to (or hidden behind) operator

In these cases (and the ones below) where this is really a fatal error, I 
believe the whole ERROR_OUT/DEBUG_EXPR/ASSERT(0) sequence should be replaced by 
a single call to panic(), which takes the same arguments as cprintf().  I think 
if you're dying you don't want to only print relevant info if you had 
traceflags enabled too.



src/mem/slicc/ast/FuncCallExprAST.py


What is this code doing?  It looks like it's inserting the location, but 
how does this work if the format string isn't changed?  Plus it would be nice 
to have a comment here in the SLICC code too.


- Steve


On 2010-10-24 08:59:56, Nilay Vaish wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/277/
> ---
> 
> (Updated 2010-10-24 08:59:56)
> 
> 
> 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
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: Moving Ruby to M5's debug print support

[Nilay Vaish]

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

(Updated 2010-10-24 08:59:56.664764)


Review request for Default and Ruby Reviewers.


Changes
---

I have made almost all the changes suggested below or on the m5-dev mailing 
list.


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 (updated)
-

  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
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: Moving Ruby to M5's debug print support

[Steve Reinhardt]

On Sat, Oct 23, 2010 at 1:59 PM, Nilay Vaish  wrote:
>
>
>> On 2010-10-21 13:35:21, Steve Reinhardt wrote:
>> > src/mem/protocol/MESI_CMP_directory-L1cache.sm, line 332
>> > 
>> >
>> >     This DPRINTF is broken (and several others like it)... it needs a 
>> > trace flag as the first arg, and "%str" probably isn't the format string 
>> > you want (basically it's just like printf, so you probably mean "%s").
>>
>> Nilay Vaish wrote:
>>     The DPRINTF statements that appear in .sm files are re-written by the 
>> SLICC compiler. SLICC compiler provides the trace flag. Looking at the 
>> format specifier, the compiler will add a .c_str() after print_str(). But in 
>> a comment above, you mentioned that c_str() is not required. May be we can 
>> drop %str altogether. I need to look in to this.
>>
>> Steve Reinhardt wrote:
>>     OK, thanks for the clarification.  I missed the SLICC compiler code 
>> before but I found it now.  I have two comments on that:
>>     - Unless there's a real need to have a different syntax, it would be 
>> nice to just write the SLICC code as a regular DPRINTF and pass DPRINTF 
>> calls through unmodified.  I'd like to better understand why you think it's 
>> needed (if you still think it's needed after making the c_str() change).
>>     - If we do end up needing a DPRINTF replacement with different syntax in 
>> SLICC, let's call it something other than DPRINTF to make it clear that it 
>> is different.
>>
>
> DPRINTF() calls will have to be modified since we want to add line number of 
> the .sm files where the call appears. I think we can maintain the syntax of 
> DPRINTF() as used in other places.

If we just want to insert __FILE__ and __LINE__ as part of the prefix,
we should define a new flavor of DPRINTF that does that in the same
header where DPRINTF itself is, and use that version as-is in SLICC.
Maybe call it DPRINTFL?

> But in order to combine adjacent DPRINTF() statements, we would need change 
> to the way DPRINTF() call is processed. As per my understanding, currently 
> SLICC compiler assumes that only one object / variable would be printed at a 
> time.

I don't understand this... if we just pass the call through verbatim,
the SLICC compiler shouldn't care at all, and even if there are minor
tweaks, I don't see why it couldn't handle a variable number of
arguments.

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

Re: [m5-dev] Review Request: Moving Ruby to M5's debug print support

[Steve Reinhardt]

On Sat, Oct 23, 2010 at 1:52 PM, Nilay Vaish  wrote:
>
>
>> On 2010-10-21 13:35:21, Steve Reinhardt wrote:
>> > src/cpu/testers/rubytest/CheckTable.cc, line 114
>> > 
>> >
>> >     These adjacent DPRINTFs should be consolidated into one.  It's more 
>> > efficient (since it's a single check of the trace flag and a single call 
>> > to cprintf), and in this case (and many others) we don't really need the 
>> > function name, file name, and line number printed redundantly.
>> >
>> >     Also you don't need to use c_str(), cprintf() handles std::string just 
>> > fine (and anything else that supports the '<<' operator, I believe).
>
> Does that mean that format specifiers used in DPRINTF() statements are just 
> place holders? In not, what would be the format specifier for printing an 
> object of some class that supports << operator?

Pretty much... if you just want DPRINTF to use '<<'  without any other
modifiers or overhead, a format string of "%s" is your best choice.

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

Re: [m5-dev] Review Request: Moving Ruby to M5's debug print support

[Nilay Vaish]

> On 2010-10-21 13:35:21, Steve Reinhardt wrote:
> > src/mem/protocol/MESI_CMP_directory-L1cache.sm, line 332
> > 
> >
> > This DPRINTF is broken (and several others like it)... it needs a trace 
> > flag as the first arg, and "%str" probably isn't the format string you want 
> > (basically it's just like printf, so you probably mean "%s").
> 
> Nilay Vaish wrote:
> The DPRINTF statements that appear in .sm files are re-written by the 
> SLICC compiler. SLICC compiler provides the trace flag. Looking at the format 
> specifier, the compiler will add a .c_str() after print_str(). But in a 
> comment above, you mentioned that c_str() is not required. May be we can drop 
> %str altogether. I need to look in to this.
> 
> Steve Reinhardt wrote:
> OK, thanks for the clarification.  I missed the SLICC compiler code 
> before but I found it now.  I have two comments on that:
> - Unless there's a real need to have a different syntax, it would be nice 
> to just write the SLICC code as a regular DPRINTF and pass DPRINTF calls 
> through unmodified.  I'd like to better understand why you think it's needed 
> (if you still think it's needed after making the c_str() change).
> - If we do end up needing a DPRINTF replacement with different syntax in 
> SLICC, let's call it something other than DPRINTF to make it clear that it is 
> different.
>

DPRINTF() calls will have to be modified since we want to add line number of 
the .sm files where the call appears. I think we can maintain the syntax of 
DPRINTF() as used in other places. But in order to combine adjacent DPRINTF() 
statements, we would need change to the way DPRINTF() call is processed. As per 
my understanding, currently SLICC compiler assumes that only one object / 
variable would be printed at a time.


- Nilay


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


On 2010-10-19 17:30:48, Nilay Vaish wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/277/
> ---
> 
> (Updated 2010-10-19 17:30:48)
> 
> 
> 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 956ac83b0a58 
>   src/mem/SConscript 956ac83b0a58 
>   src/mem/protocol/MESI_CMP_directory-L1cache.sm 956ac83b0a58 
>   src/mem/protocol/MESI_CMP_directory-L2cache.sm 956ac83b0a58 
>   src/mem/protocol/MESI_CMP_directory-dir.sm 956ac83b0a58 
>   src/mem/protocol/MI_example-cache.sm 956ac83b0a58 
>   src/mem/protocol/MI_example-dir.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_directory-L1cache.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_directory-L2cache.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_directory-dir.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_directory-perfectDir.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_token-L1cache.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_token-L2cache.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_token-dir.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_hammer-cache.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_hammer-dir.sm 956ac83b0a58 
>   src/mem/protocol/RubySlicc_Exports.sm 956ac83b0a58 
>   src/mem/protocol/RubySlicc_MemControl.sm 956ac83b0a58 
>   src/mem/protocol/RubySlicc_Types.sm 956ac83b0a58 
>   src/mem/ruby/SConsopts 956ac83b0a58 
>   src/mem/ruby/buffers/MessageBuffer.cc 956ac83b0a58 
>   src/mem/ruby/common/Address.hh 956ac83b0a58 
>   src/mem/ruby/common/DataBlock.hh 956ac83b0a58 
>   src/mem/ruby/common/Debug.hh 956ac83b0a58 
>   src/mem/ruby/common/Debug.cc 956ac83b0a58 
>   src/mem/ruby/common/NetDest.hh 956ac83b0a58 
>   src/mem/ruby/common/NetDest.cc 956ac83b0a58 
>   src/mem/ruby/network/garnet/fixed-pipeline/NetworkInterface_d.cc 
> 956ac83b0a58 
>   src/mem/ruby/network/garnet/fixed-pipeline/Switch_d.cc 956ac83b0a58 
>   src/mem/ruby/network/garnet/flexible-pipeline/NetworkInterface.cc 
> 956ac83b0a58 
>   src/mem/ruby/network/garnet/flexible-pipeline/Router.cc 956ac83b0a58 
>   src/mem/ruby/network/simple/PerfectSwitch.cc 956ac83b0a58 
>   src/mem/ruby/network/simple/Throttle.cc 956ac83b0a58 
>   src/mem/ruby/network/simple/Topology.cc 956ac83b0a58 
>   src/mem/ruby/slicc_interface/Message.hh 956ac83b0a58 
>   src/mem/ruby/slicc_interface/NetworkMessage.hh 956ac

Re: [m5-dev] Review Request: Moving Ruby to M5's debug print support

[Nilay Vaish]

> On 2010-10-21 13:35:21, Steve Reinhardt wrote:
> > src/cpu/testers/rubytest/CheckTable.cc, line 114
> > 
> >
> > These adjacent DPRINTFs should be consolidated into one.  It's more 
> > efficient (since it's a single check of the trace flag and a single call to 
> > cprintf), and in this case (and many others) we don't really need the 
> > function name, file name, and line number printed redundantly.
> > 
> > Also you don't need to use c_str(), cprintf() handles std::string just 
> > fine (and anything else that supports the '<<' operator, I believe).

Does that mean that format specifiers used in DPRINTF() statements are just 
place holders? In not, what would be the format specifier for printing an 
object of some class that supports << operator?


- Nilay


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


On 2010-10-19 17:30:48, Nilay Vaish wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/277/
> ---
> 
> (Updated 2010-10-19 17:30:48)
> 
> 
> 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 956ac83b0a58 
>   src/mem/SConscript 956ac83b0a58 
>   src/mem/protocol/MESI_CMP_directory-L1cache.sm 956ac83b0a58 
>   src/mem/protocol/MESI_CMP_directory-L2cache.sm 956ac83b0a58 
>   src/mem/protocol/MESI_CMP_directory-dir.sm 956ac83b0a58 
>   src/mem/protocol/MI_example-cache.sm 956ac83b0a58 
>   src/mem/protocol/MI_example-dir.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_directory-L1cache.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_directory-L2cache.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_directory-dir.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_directory-perfectDir.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_token-L1cache.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_token-L2cache.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_token-dir.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_hammer-cache.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_hammer-dir.sm 956ac83b0a58 
>   src/mem/protocol/RubySlicc_Exports.sm 956ac83b0a58 
>   src/mem/protocol/RubySlicc_MemControl.sm 956ac83b0a58 
>   src/mem/protocol/RubySlicc_Types.sm 956ac83b0a58 
>   src/mem/ruby/SConsopts 956ac83b0a58 
>   src/mem/ruby/buffers/MessageBuffer.cc 956ac83b0a58 
>   src/mem/ruby/common/Address.hh 956ac83b0a58 
>   src/mem/ruby/common/DataBlock.hh 956ac83b0a58 
>   src/mem/ruby/common/Debug.hh 956ac83b0a58 
>   src/mem/ruby/common/Debug.cc 956ac83b0a58 
>   src/mem/ruby/common/NetDest.hh 956ac83b0a58 
>   src/mem/ruby/common/NetDest.cc 956ac83b0a58 
>   src/mem/ruby/network/garnet/fixed-pipeline/NetworkInterface_d.cc 
> 956ac83b0a58 
>   src/mem/ruby/network/garnet/fixed-pipeline/Switch_d.cc 956ac83b0a58 
>   src/mem/ruby/network/garnet/flexible-pipeline/NetworkInterface.cc 
> 956ac83b0a58 
>   src/mem/ruby/network/garnet/flexible-pipeline/Router.cc 956ac83b0a58 
>   src/mem/ruby/network/simple/PerfectSwitch.cc 956ac83b0a58 
>   src/mem/ruby/network/simple/Throttle.cc 956ac83b0a58 
>   src/mem/ruby/network/simple/Topology.cc 956ac83b0a58 
>   src/mem/ruby/slicc_interface/Message.hh 956ac83b0a58 
>   src/mem/ruby/slicc_interface/NetworkMessage.hh 956ac83b0a58 
>   src/mem/ruby/storebuffer/storebuffer.cc 956ac83b0a58 
>   src/mem/ruby/system/CacheMemory.cc 956ac83b0a58 
>   src/mem/ruby/system/DirectoryMemory.cc 956ac83b0a58 
>   src/mem/ruby/system/SConscript 956ac83b0a58 
>   src/mem/ruby/system/SparseMemory.cc 956ac83b0a58 
>   src/mem/ruby/tester/RaceyPseudoThread.cc 956ac83b0a58 
>   src/mem/slicc/ast/FuncCallExprAST.py 956ac83b0a58 
>   src/mem/slicc/symbols/StateMachine.py 956ac83b0a58 
>   src/mem/slicc/symbols/Type.py 956ac83b0a58 
> 
> Diff: http://reviews.m5sim.org/r/277/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Nilay
> 
>

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

Re: [m5-dev] Review Request: Moving Ruby to M5's debug print support

[Beckmann, Brad]

I see.  When I suggested to Nilay to implement the print functions and remove 
the operator<< functions, I was using M5's pkt print statements as loose 
inspiration.  I should have been more thorough in my investigation because I 
know realize that there are a few M5 base objects that do the operator<< 
overloading.  Fortunately, it should be pretty easy for Nilay to change his 
current print functions to operator<< ccprintf functions. 

Brad


-Original Message-
From: m5-dev-boun...@m5sim.org [mailto:m5-dev-boun...@m5sim.org] On Behalf Of 
Steve Reinhardt
Sent: Friday, October 22, 2010 3:38 PM
To: M5 Developer List
Subject: Re: [m5-dev] Review Request: Moving Ruby to M5's debug print support

On Fri, Oct 22, 2010 at 3:12 PM, nathan binkert  wrote:
>> So I guess the upshot is that if there are complex Ruby types (not 
>> just typedefs of builtin types) that want to have standard ways of 
>> printing themselves out, that's kind of inconsistent with M5 right 
>> there :-), but if you want to keep that behavior then overloading 
>> operator<< is the way to do it.
>
> Gabe is adding a PC type though and I believe he's overloading 
> operator<<.  I generally think that this is a pretty nice thing to do.
>  It's more or less analagous to implementing __str__ in python which 
> is a pretty nice thing to have, so I'd say that Ruby is pushing a good 
> idea into M5.

Yea, I didn't mean to imply that M5's way was better, just that Brad's 
observation of Ruby's inconsistency with M5 was maybe focused on a symptom and 
not on the cause.  There probably are places where M5's debug output could be 
improved by using operator<<,  for example it would be nice to standardize on 
whether we print 0x in front of addresses in cache traces.  Ruby's practice of 
always printing both the byte address and the block address is also 
interesting, though personally I find that pretty redundant and I've become 
practiced at creating regexes that can capture all the byte addresses within a 
block so it's pretty easy to egrep out the trace entries that relate to a 
particular block.

Steve
___
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: Moving Ruby to M5's debug print support

[Steve Reinhardt]

On Fri, Oct 22, 2010 at 3:12 PM, nathan binkert  wrote:
>> So I guess the upshot is that if there are complex Ruby types (not
>> just typedefs of builtin types) that want to have standard ways of
>> printing themselves out, that's kind of inconsistent with M5 right
>> there :-), but if you want to keep that behavior then overloading
>> operator<< is the way to do it.
>
> Gabe is adding a PC type though and I believe he's overloading
> operator<<.  I generally think that this is a pretty nice thing to do.
>  It's more or less analagous to implementing __str__ in python which
> is a pretty nice thing to have, so I'd say that Ruby is pushing a good
> idea into M5.

Yea, I didn't mean to imply that M5's way was better, just that Brad's
observation of Ruby's inconsistency with M5 was maybe focused on a
symptom and not on the cause.  There probably are places where M5's
debug output could be improved by using operator<<,  for example it
would be nice to standardize on whether we print 0x in front of
addresses in cache traces.  Ruby's practice of always printing both
the byte address and the block address is also interesting, though
personally I find that pretty redundant and I've become practiced at
creating regexes that can capture all the byte addresses within a
block so it's pretty easy to egrep out the trace entries that relate
to a particular block.

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

Re: [m5-dev] Review Request: Moving Ruby to M5's debug print support

[Gabriel Michael Black]

One exception to that are the PCState structs I'm in the process of  
adding. These each overload << to print themselves as  
(X1=>X2=>X3).(Y1=>Y2) where the X*s are as many architecture level PCs  
as are needed and the Y*s are microPCs. This is in the middle where  
it's too complex for a single built in data type but still simple  
enough to be printed as a unit. This also allows ISA independent code  
to do things like:


DPRINTF(Fetch, "The PC is %s.\n", pc);

without having to know what's in the pc or how to print its components.

Generally, though, I agree with Steve that there's a pretty narrow  
window where this sort of thing makes sense and meshes at least  
stylistically with the rest of M5.


Gabe

Quoting Steve Reinhardt :


I think part of the confusion is that we don't typically overload
operator<< in M5 because we don't typically define complex types that
have standard ways of printing themselves out.  So for example simple
M5 types like Tick and Addr are just typedefs for uint64_t (or
something like that), so << just works on them.  More complex objects
are just too complex to have simple string representations, or at
least we haven't felt the need to standardize them, so we just have
ad-hoc DPRINTFs that print out the relevant scalar fields as needed.

So I guess the upshot is that if there are complex Ruby types (not
just typedefs of builtin types) that want to have standard ways of
printing themselves out, that's kind of inconsistent with M5 right
there :-), but if you want to keep that behavior then overloading
operator<< is the way to do it.

Steve
___
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: Moving Ruby to M5's debug print support

[nathan binkert]

> So I guess the upshot is that if there are complex Ruby types (not
> just typedefs of builtin types) that want to have standard ways of
> printing themselves out, that's kind of inconsistent with M5 right
> there :-), but if you want to keep that behavior then overloading
> operator<< is the way to do it.

Gabe is adding a PC type though and I believe he's overloading
operator<<.  I generally think that this is a pretty nice thing to do.
 It's more or less analagous to implementing __str__ in python which
is a pretty nice thing to have, so I'd say that Ruby is pushing a good
idea into M5.

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

Re: [m5-dev] Review Request: Moving Ruby to M5's debug print support

[Steve Reinhardt]

I think part of the confusion is that we don't typically overload
operator<< in M5 because we don't typically define complex types that
have standard ways of printing themselves out.  So for example simple
M5 types like Tick and Addr are just typedefs for uint64_t (or
something like that), so << just works on them.  More complex objects
are just too complex to have simple string representations, or at
least we haven't felt the need to standardize them, so we just have
ad-hoc DPRINTFs that print out the relevant scalar fields as needed.

So I guess the upshot is that if there are complex Ruby types (not
just typedefs of builtin types) that want to have standard ways of
printing themselves out, that's kind of inconsistent with M5 right
there :-), but if you want to keep that behavior then overloading
operator<< is the way to do it.

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

Re: [m5-dev] Review Request: Moving Ruby to M5's debug print support

[nathan binkert]

One additional comment, there is no reason that the implementation of
operator<< couldn't itself use cprintf.  e.g.:

iostream &
operator<<(iostream &out, const Foo &foo)
{
ccprintf(out, "<%d:%s>", foo.val, foo.name());
return out;
}

On Fri, Oct 22, 2010 at 2:29 PM, nathan binkert  wrote:
>> Nilay, I apologize for the confusion here.  It seems that Nate and I are 
>> suggesting you go in two different directions in terms of the operator<<.  I 
>> personally wanted to get away from the operator<< overloading, so that Ruby 
>> objects looked more similar to most M5 objects.  Especially because most of 
>> these operator<< functions aren't very useful.
>>
>> Nate, could you elaborate why you'd prefer the operator<< functions versus 
>> the print functions?  My motivation for removing the operator<< functions 
>> was to improve our consistency, but if you have a better reason for removing 
>> the print functions instead, I'm willing to change my mind.
>
> As Steve said, at the bottom of cprintf/DPRINTF, the system uses
> operator<<, so if you use %s, it can handle printing it.  For example.
>  If you had an object Foo that overloaded operator<< and an instance
> foo of that object, you could simply do
> DPRINTF(Flag, "%s", foo)
>
> Instead of what's happening with what you have now of doing:
> DPRINTF(Flag, "%s", foo.print_str());
>
> Furthermore, the printing will be a lot faster with operator<< and
> it's more versatile.
>
> In my opinion, print/print_str should both be gotten rid of and
> operator<< should be what we use.
>
> In short.  operator<< is good.  Using it with cout is bad.
>
> Make sense?
>
>  Nate
>
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: Moving Ruby to M5's debug print support

[nathan binkert]

> Nilay, I apologize for the confusion here.  It seems that Nate and I are 
> suggesting you go in two different directions in terms of the operator<<.  I 
> personally wanted to get away from the operator<< overloading, so that Ruby 
> objects looked more similar to most M5 objects.  Especially because most of 
> these operator<< functions aren't very useful.
>
> Nate, could you elaborate why you'd prefer the operator<< functions versus 
> the print functions?  My motivation for removing the operator<< functions was 
> to improve our consistency, but if you have a better reason for removing the 
> print functions instead, I'm willing to change my mind.

As Steve said, at the bottom of cprintf/DPRINTF, the system uses
operator<<, so if you use %s, it can handle printing it.  For example.
 If you had an object Foo that overloaded operator<< and an instance
foo of that object, you could simply do
DPRINTF(Flag, "%s", foo)

Instead of what's happening with what you have now of doing:
DPRINTF(Flag, "%s", foo.print_str());

Furthermore, the printing will be a lot faster with operator<< and
it's more versatile.

In my opinion, print/print_str should both be gotten rid of and
operator<< should be what we use.

In short.  operator<< is good.  Using it with cout is bad.

Make sense?

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

Re: [m5-dev] Review Request: Moving Ruby to M5's debug print support

[Beckmann, Brad]

Nilay, I apologize for the confusion here.  It seems that Nate and I are 
suggesting you go in two different directions in terms of the operator<<.  I 
personally wanted to get away from the operator<< overloading, so that Ruby 
objects looked more similar to most M5 objects.  Especially because most of 
these operator<< functions aren't very useful.

Nate, could you elaborate why you'd prefer the operator<< functions versus the 
print functions?  My motivation for removing the operator<< functions was to 
improve our consistency, but if you have a better reason for removing the print 
functions instead, I'm willing to change my mind.

Brad


-Original Message-
From: m5-dev-boun...@m5sim.org [mailto:m5-dev-boun...@m5sim.org] On Behalf Of 
Nathan Binkert
Sent: Friday, October 22, 2010 9:54 AM
To: Nilay Vaish; Default; Ruby Reviewers; Nathan Binkert
Subject: Re: [m5-dev] Review Request: Moving Ruby to M5's debug print support


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

Ship it!


Overall, I have two major comments.  1) You should get rid of print_str() (and 
probably print()) for that matter and implement operator<<.  2) You shouldn't 
be duplicating so many lines with the __PRETTY_FUNCTION__, __FILE__, __LINE__ 
stuff.  I think it should be buried in a macro.  In fact, if people like, we 
could make those available to *ALL* DPRINTFs and make turning them on and off 
another trace flag.  In all honesty though, they tend not to be necessary if 
your trace strings are at all descriptive.


src/cpu/testers/rubytest/CheckTable.cc
<http://reviews.m5sim.org/r/277/#comment551>

You don't need __PRETTY_FUNCTION__ and __FILE__/__LINE__.  I'd say that one 
or the other should do.  Also if you're going to do this a lot, we should have 
a new DPRINTF macro for it.



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

I would emphatically argue *no*



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

I agree that we should pass it through unmodified.

Also, DPRINTF understands operator<<, and if you use %s, it can use any 
object with an operator<<, so I'd get rid of print_str() on the objects and 
just rename it operator<<



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

I agree with Steve.  Commented out code is a no-no.



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

Can you consolidate these into a single expression that has some context?  
I guess that's not the point of this diff though, so I don't think it's 
required.



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

Same here



src/mem/protocol/RubySlicc_Exports.sm
<http://reviews.m5sim.org/r/277/#comment558>

We should probably change this to operator<<, then again, I am not quite 
clear how ruby uses this.




src/mem/ruby/SConsopts
<http://reviews.m5sim.org/r/277/#comment559>

Good work!



src/mem/ruby/buffers/MessageBuffer.cc
<http://reviews.m5sim.org/r/277/#comment560>

I'd rather see a new DPRINT Macro.



src/mem/ruby/buffers/MessageBuffer.cc
<http://reviews.m5sim.org/r/277/#comment561>

Generally, this is a bad idea.



src/mem/ruby/buffers/MessageBuffer.cc
<http://reviews.m5sim.org/r/277/#comment562>

you don't need to use c_str(), and in fact name() is automatically inserted 
into all DPRINTF statements.



src/mem/ruby/buffers/MessageBuffer.cc
<http://reviews.m5sim.org/r/277/#comment563>

why is "message" in the argument list?  Also, why this very complicated 
expression to get some message string?  I suggest getting rid of print_str() 
and making operator<< work.



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

I'd honestly get rid of both print and print_str and just simply implement 
operator<<



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

Yes.  That is exactly correct.



src/mem/ruby/network/simple/PerfectSwitch.cc
<http://reviews.m5sim.org/r/277/#comment566>

Pretty strange that you keep having things like "m_switch_id" in the 
argument list.



src/mem/ruby/storebuffer/storebuffer.cc
<http://reviews.m5sim.org/r/277/#comment567>

Instead of commenting these out, you could add a new Trace flag for these 
and get rid of the #ifdefs.  Not sure if it's useful or not though.


- Nathan


On 2010-10-19 17:30:48, Nilay Vaish wrote:
> 
> --

Re: [m5-dev] Review Request: Moving Ruby to M5's debug print support

[Nathan Binkert]

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

Ship it!


Overall, I have two major comments.  1) You should get rid of print_str() (and 
probably print()) for that matter and implement operator<<.  2) You shouldn't 
be duplicating so many lines with the __PRETTY_FUNCTION__, __FILE__, __LINE__ 
stuff.  I think it should be buried in a macro.  In fact, if people like, we 
could make those available to *ALL* DPRINTFs and make turning them on and off 
another trace flag.  In all honesty though, they tend not to be necessary if 
your trace strings are at all descriptive.


src/cpu/testers/rubytest/CheckTable.cc


You don't need __PRETTY_FUNCTION__ and __FILE__/__LINE__.  I'd say that one 
or the other should do.  Also if you're going to do this a lot, we should have 
a new DPRINTF macro for it.



src/mem/SConscript


I would emphatically argue *no*



src/mem/protocol/MESI_CMP_directory-L1cache.sm


I agree that we should pass it through unmodified.

Also, DPRINTF understands operator<<, and if you use %s, it can use any 
object with an operator<<, so I'd get rid of print_str() on the objects and 
just rename it operator<<



src/mem/protocol/MESI_CMP_directory-L1cache.sm


I agree with Steve.  Commented out code is a no-no.



src/mem/protocol/MESI_CMP_directory-L2cache.sm


Can you consolidate these into a single expression that has some context?  
I guess that's not the point of this diff though, so I don't think it's 
required.



src/mem/protocol/MESI_CMP_directory-L2cache.sm


Same here



src/mem/protocol/RubySlicc_Exports.sm


We should probably change this to operator<<, then again, I am not quite 
clear how ruby uses this.




src/mem/ruby/SConsopts


Good work!



src/mem/ruby/buffers/MessageBuffer.cc


I'd rather see a new DPRINT Macro.



src/mem/ruby/buffers/MessageBuffer.cc


Generally, this is a bad idea.



src/mem/ruby/buffers/MessageBuffer.cc


you don't need to use c_str(), and in fact name() is automatically inserted 
into all DPRINTF statements.



src/mem/ruby/buffers/MessageBuffer.cc


why is "message" in the argument list?  Also, why this very complicated 
expression to get some message string?  I suggest getting rid of print_str() 
and making operator<< work.



src/mem/ruby/common/Address.hh


I'd honestly get rid of both print and print_str and just simply implement 
operator<<



src/mem/ruby/common/Address.hh


Yes.  That is exactly correct.



src/mem/ruby/network/simple/PerfectSwitch.cc


Pretty strange that you keep having things like "m_switch_id" in the 
argument list.



src/mem/ruby/storebuffer/storebuffer.cc


Instead of commenting these out, you could add a new Trace flag for these 
and get rid of the #ifdefs.  Not sure if it's useful or not though.


- Nathan


On 2010-10-19 17:30:48, Nilay Vaish wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/277/
> ---
> 
> (Updated 2010-10-19 17:30:48)
> 
> 
> 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 956ac83b0a58 
>   src/mem/SConscript 956ac83b0a58 
>   src/mem/protocol/MESI_CMP_directory-L1cache.sm 956ac83b0a58 
>   src/mem/protocol/MESI_CMP_directory-L2cache.sm 956ac83b0a58 
>   src/mem/protocol/MESI_CMP_directory-dir.sm 956ac83b0a58 
>   src/mem/protocol/MI_example-cache.sm 956ac83b0a58 
>   src/mem/protocol/MI_example-dir.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_directory-L1cache.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_directory-L2cache.sm 956ac83b0a58

Re: [m5-dev] Review Request: Moving Ruby to M5's debug print support

[Steve Reinhardt]

> On 2010-10-21 13:35:21, Steve Reinhardt wrote:
> > src/mem/protocol/MESI_CMP_directory-L1cache.sm, line 333
> > 
> >
> > We should get rid of these commented-out lines, either deleting them if 
> > they're not needed or just using a trace flag that keeps them turned off 
> > unless wanted.  That's the point of the trace flags, is so that you don't 
> > have to comment out debug printfs, but can just turn them on and off 
> > dynamically at the granularity you want.
> 
> Nilay Vaish wrote:
> I can remove the commented lines, but I may not be the right person to 
> make that decision.

I would lean toward uncommenting them rather than removing them, and then if 
people later find them too verbose they can tweak the trace flags to turn them 
on more selectively.  Note that in some cases we have a Foo flag and a 
FooVerbose flag to get different levels of tracing on the same category of 
DPRINTFs... not that we need to do that here, but it's an option.  In the cases 
where the commented-out lines are adjacent to uncommented lines, I expect they 
could be condensed into a single DPRINTF.


- Steve


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


On 2010-10-19 17:30:48, Nilay Vaish wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/277/
> ---
> 
> (Updated 2010-10-19 17:30:48)
> 
> 
> 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 956ac83b0a58 
>   src/mem/SConscript 956ac83b0a58 
>   src/mem/protocol/MESI_CMP_directory-L1cache.sm 956ac83b0a58 
>   src/mem/protocol/MESI_CMP_directory-L2cache.sm 956ac83b0a58 
>   src/mem/protocol/MESI_CMP_directory-dir.sm 956ac83b0a58 
>   src/mem/protocol/MI_example-cache.sm 956ac83b0a58 
>   src/mem/protocol/MI_example-dir.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_directory-L1cache.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_directory-L2cache.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_directory-dir.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_directory-perfectDir.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_token-L1cache.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_token-L2cache.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_token-dir.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_hammer-cache.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_hammer-dir.sm 956ac83b0a58 
>   src/mem/protocol/RubySlicc_Exports.sm 956ac83b0a58 
>   src/mem/protocol/RubySlicc_MemControl.sm 956ac83b0a58 
>   src/mem/protocol/RubySlicc_Types.sm 956ac83b0a58 
>   src/mem/ruby/SConsopts 956ac83b0a58 
>   src/mem/ruby/buffers/MessageBuffer.cc 956ac83b0a58 
>   src/mem/ruby/common/Address.hh 956ac83b0a58 
>   src/mem/ruby/common/DataBlock.hh 956ac83b0a58 
>   src/mem/ruby/common/Debug.hh 956ac83b0a58 
>   src/mem/ruby/common/Debug.cc 956ac83b0a58 
>   src/mem/ruby/common/NetDest.hh 956ac83b0a58 
>   src/mem/ruby/common/NetDest.cc 956ac83b0a58 
>   src/mem/ruby/network/garnet/fixed-pipeline/NetworkInterface_d.cc 
> 956ac83b0a58 
>   src/mem/ruby/network/garnet/fixed-pipeline/Switch_d.cc 956ac83b0a58 
>   src/mem/ruby/network/garnet/flexible-pipeline/NetworkInterface.cc 
> 956ac83b0a58 
>   src/mem/ruby/network/garnet/flexible-pipeline/Router.cc 956ac83b0a58 
>   src/mem/ruby/network/simple/PerfectSwitch.cc 956ac83b0a58 
>   src/mem/ruby/network/simple/Throttle.cc 956ac83b0a58 
>   src/mem/ruby/network/simple/Topology.cc 956ac83b0a58 
>   src/mem/ruby/slicc_interface/Message.hh 956ac83b0a58 
>   src/mem/ruby/slicc_interface/NetworkMessage.hh 956ac83b0a58 
>   src/mem/ruby/storebuffer/storebuffer.cc 956ac83b0a58 
>   src/mem/ruby/system/CacheMemory.cc 956ac83b0a58 
>   src/mem/ruby/system/DirectoryMemory.cc 956ac83b0a58 
>   src/mem/ruby/system/SConscript 956ac83b0a58 
>   src/mem/ruby/system/SparseMemory.cc 956ac83b0a58 
>   src/mem/ruby/tester/RaceyPseudoThread.cc 956ac83b0a58 
>   src/mem/slicc/ast/FuncCallExprAST.py 956ac83b0a58 
>   src/mem/slicc/symbols/StateMachine.py 956ac83b0a58 
>   src/mem/slicc/symbols/Type.py 956ac83b0a58 
> 
> Diff: http://reviews.m5sim.org/r/277/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Nilay
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
htt

Re: [m5-dev] Review Request: Moving Ruby to M5's debug print support

[Steve Reinhardt]

> On 2010-10-21 13:35:21, Steve Reinhardt wrote:
> > src/mem/protocol/MESI_CMP_directory-L1cache.sm, line 332
> > 
> >
> > This DPRINTF is broken (and several others like it)... it needs a trace 
> > flag as the first arg, and "%str" probably isn't the format string you want 
> > (basically it's just like printf, so you probably mean "%s").
> 
> Nilay Vaish wrote:
> The DPRINTF statements that appear in .sm files are re-written by the 
> SLICC compiler. SLICC compiler provides the trace flag. Looking at the format 
> specifier, the compiler will add a .c_str() after print_str(). But in a 
> comment above, you mentioned that c_str() is not required. May be we can drop 
> %str altogether. I need to look in to this.

OK, thanks for the clarification.  I missed the SLICC compiler code before but 
I found it now.  I have two comments on that:
- Unless there's a real need to have a different syntax, it would be nice to 
just write the SLICC code as a regular DPRINTF and pass DPRINTF calls through 
unmodified.  I'd like to better understand why you think it's needed (if you 
still think it's needed after making the c_str() change).
- If we do end up needing a DPRINTF replacement with different syntax in SLICC, 
let's call it something other than DPRINTF to make it clear that it is 
different.


- Steve


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


On 2010-10-19 17:30:48, Nilay Vaish wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/277/
> ---
> 
> (Updated 2010-10-19 17:30:48)
> 
> 
> 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 956ac83b0a58 
>   src/mem/SConscript 956ac83b0a58 
>   src/mem/protocol/MESI_CMP_directory-L1cache.sm 956ac83b0a58 
>   src/mem/protocol/MESI_CMP_directory-L2cache.sm 956ac83b0a58 
>   src/mem/protocol/MESI_CMP_directory-dir.sm 956ac83b0a58 
>   src/mem/protocol/MI_example-cache.sm 956ac83b0a58 
>   src/mem/protocol/MI_example-dir.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_directory-L1cache.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_directory-L2cache.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_directory-dir.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_directory-perfectDir.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_token-L1cache.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_token-L2cache.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_token-dir.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_hammer-cache.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_hammer-dir.sm 956ac83b0a58 
>   src/mem/protocol/RubySlicc_Exports.sm 956ac83b0a58 
>   src/mem/protocol/RubySlicc_MemControl.sm 956ac83b0a58 
>   src/mem/protocol/RubySlicc_Types.sm 956ac83b0a58 
>   src/mem/ruby/SConsopts 956ac83b0a58 
>   src/mem/ruby/buffers/MessageBuffer.cc 956ac83b0a58 
>   src/mem/ruby/common/Address.hh 956ac83b0a58 
>   src/mem/ruby/common/DataBlock.hh 956ac83b0a58 
>   src/mem/ruby/common/Debug.hh 956ac83b0a58 
>   src/mem/ruby/common/Debug.cc 956ac83b0a58 
>   src/mem/ruby/common/NetDest.hh 956ac83b0a58 
>   src/mem/ruby/common/NetDest.cc 956ac83b0a58 
>   src/mem/ruby/network/garnet/fixed-pipeline/NetworkInterface_d.cc 
> 956ac83b0a58 
>   src/mem/ruby/network/garnet/fixed-pipeline/Switch_d.cc 956ac83b0a58 
>   src/mem/ruby/network/garnet/flexible-pipeline/NetworkInterface.cc 
> 956ac83b0a58 
>   src/mem/ruby/network/garnet/flexible-pipeline/Router.cc 956ac83b0a58 
>   src/mem/ruby/network/simple/PerfectSwitch.cc 956ac83b0a58 
>   src/mem/ruby/network/simple/Throttle.cc 956ac83b0a58 
>   src/mem/ruby/network/simple/Topology.cc 956ac83b0a58 
>   src/mem/ruby/slicc_interface/Message.hh 956ac83b0a58 
>   src/mem/ruby/slicc_interface/NetworkMessage.hh 956ac83b0a58 
>   src/mem/ruby/storebuffer/storebuffer.cc 956ac83b0a58 
>   src/mem/ruby/system/CacheMemory.cc 956ac83b0a58 
>   src/mem/ruby/system/DirectoryMemory.cc 956ac83b0a58 
>   src/mem/ruby/system/SConscript 956ac83b0a58 
>   src/mem/ruby/system/SparseMemory.cc 956ac83b0a58 
>   src/mem/ruby/tester/RaceyPseudoThread.cc 956ac83b0a58 
>   src/mem/slicc/ast/FuncCallExprAST.py 956ac83b0a58 
>   src/mem/slicc/symbols/StateMachine.py 956ac83b0a58 
>   src/mem/slicc/symbols/Type.py 956

Re: [m5-dev] Review Request: Moving Ruby to M5's debug print support

[Nilay Vaish]

> On 2010-10-21 15:23:04, Brad Beckmann wrote:
> > src/mem/ruby/common/Debug.hh, line 231
> > 
> >
> > I agree with Steve's point that adding "__PRETTY_FUNCTION__, __FILE__, 
> > __LINE__" to every Ruby DPRINTF call is too cumbersome and we either need 
> > to remove it or define a macro that does it automatically.  Nilay, I think 
> > I might have confused you during our previous email conversations.  We 
> > definitely want to automatically include the function name, file, and line 
> > number for the SLICC/.sm debug statements...I've found that feature very 
> > useful in the past.  However, I'm not sure we need it for the debug 
> > statements on the Ruby side.  Nilay, if I understand your changes to SLICC 
> > correctly and recall from our previous conversations on this topic, these 
> > old ruby DEBUG macros relied on the ostream operator<< overloading, 
> > correct? That is why you now have the conditional statements in the SLICC 
> > that generate the correct DPRINTF line depending on the type.  Am I 
> > understanding the issues correctly?  If so, I think your solution for the 
> > SLICC generation of DPRINTF statements loo
 ks good.  I just think that we need to clean up the Ruby DPRINTF statements.
> >

I will remove the function name, file name, line number from Ruby DPRINTF 
statements.


- Nilay


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


On 2010-10-19 17:30:48, Nilay Vaish wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/277/
> ---
> 
> (Updated 2010-10-19 17:30:48)
> 
> 
> 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 956ac83b0a58 
>   src/mem/SConscript 956ac83b0a58 
>   src/mem/protocol/MESI_CMP_directory-L1cache.sm 956ac83b0a58 
>   src/mem/protocol/MESI_CMP_directory-L2cache.sm 956ac83b0a58 
>   src/mem/protocol/MESI_CMP_directory-dir.sm 956ac83b0a58 
>   src/mem/protocol/MI_example-cache.sm 956ac83b0a58 
>   src/mem/protocol/MI_example-dir.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_directory-L1cache.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_directory-L2cache.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_directory-dir.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_directory-perfectDir.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_token-L1cache.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_token-L2cache.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_token-dir.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_hammer-cache.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_hammer-dir.sm 956ac83b0a58 
>   src/mem/protocol/RubySlicc_Exports.sm 956ac83b0a58 
>   src/mem/protocol/RubySlicc_MemControl.sm 956ac83b0a58 
>   src/mem/protocol/RubySlicc_Types.sm 956ac83b0a58 
>   src/mem/ruby/SConsopts 956ac83b0a58 
>   src/mem/ruby/buffers/MessageBuffer.cc 956ac83b0a58 
>   src/mem/ruby/common/Address.hh 956ac83b0a58 
>   src/mem/ruby/common/DataBlock.hh 956ac83b0a58 
>   src/mem/ruby/common/Debug.hh 956ac83b0a58 
>   src/mem/ruby/common/Debug.cc 956ac83b0a58 
>   src/mem/ruby/common/NetDest.hh 956ac83b0a58 
>   src/mem/ruby/common/NetDest.cc 956ac83b0a58 
>   src/mem/ruby/network/garnet/fixed-pipeline/NetworkInterface_d.cc 
> 956ac83b0a58 
>   src/mem/ruby/network/garnet/fixed-pipeline/Switch_d.cc 956ac83b0a58 
>   src/mem/ruby/network/garnet/flexible-pipeline/NetworkInterface.cc 
> 956ac83b0a58 
>   src/mem/ruby/network/garnet/flexible-pipeline/Router.cc 956ac83b0a58 
>   src/mem/ruby/network/simple/PerfectSwitch.cc 956ac83b0a58 
>   src/mem/ruby/network/simple/Throttle.cc 956ac83b0a58 
>   src/mem/ruby/network/simple/Topology.cc 956ac83b0a58 
>   src/mem/ruby/slicc_interface/Message.hh 956ac83b0a58 
>   src/mem/ruby/slicc_interface/NetworkMessage.hh 956ac83b0a58 
>   src/mem/ruby/storebuffer/storebuffer.cc 956ac83b0a58 
>   src/mem/ruby/system/CacheMemory.cc 956ac83b0a58 
>   src/mem/ruby/system/DirectoryMemory.cc 956ac83b0a58 
>   src/mem/ruby/system/SConscript 956ac83b0a58 
>   src/mem/ruby/system/SparseMemory.cc 956ac83b0a58 
>   src/mem/ruby/tester/RaceyPseudoThread.cc 956ac83b0a58 
>   src/mem/slicc/ast/FuncCallExprAST.py 956ac83b0a58 
>   src/mem/slicc/symbols/StateMachine.py 956ac83b0a58 
>   src/mem/slicc/symbols/T

Re: [m5-dev] Review Request: Moving Ruby to M5's debug print support

[Nilay Vaish]

> On 2010-10-21 13:35:21, Steve Reinhardt wrote:
> > src/mem/protocol/MESI_CMP_directory-L1cache.sm, line 332
> > 
> >
> > This DPRINTF is broken (and several others like it)... it needs a trace 
> > flag as the first arg, and "%str" probably isn't the format string you want 
> > (basically it's just like printf, so you probably mean "%s").

The DPRINTF statements that appear in .sm files are re-written by the SLICC 
compiler. SLICC compiler provides the trace flag. Looking at the format 
specifier, the compiler will add a .c_str() after print_str(). But in a comment 
above, you mentioned that c_str() is not required. May be we can drop %str 
altogether. I need to look in to this.


> On 2010-10-21 13:35:21, Steve Reinhardt wrote:
> > src/mem/protocol/MESI_CMP_directory-L1cache.sm, line 333
> > 
> >
> > We should get rid of these commented-out lines, either deleting them if 
> > they're not needed or just using a trace flag that keeps them turned off 
> > unless wanted.  That's the point of the trace flags, is so that you don't 
> > have to comment out debug printfs, but can just turn them on and off 
> > dynamically at the granularity you want.

I can remove the commented lines, but I may not be the right person to make 
that decision.


- Nilay


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


On 2010-10-19 17:30:48, Nilay Vaish wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/277/
> ---
> 
> (Updated 2010-10-19 17:30:48)
> 
> 
> 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 956ac83b0a58 
>   src/mem/SConscript 956ac83b0a58 
>   src/mem/protocol/MESI_CMP_directory-L1cache.sm 956ac83b0a58 
>   src/mem/protocol/MESI_CMP_directory-L2cache.sm 956ac83b0a58 
>   src/mem/protocol/MESI_CMP_directory-dir.sm 956ac83b0a58 
>   src/mem/protocol/MI_example-cache.sm 956ac83b0a58 
>   src/mem/protocol/MI_example-dir.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_directory-L1cache.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_directory-L2cache.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_directory-dir.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_directory-perfectDir.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_token-L1cache.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_token-L2cache.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_token-dir.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_hammer-cache.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_hammer-dir.sm 956ac83b0a58 
>   src/mem/protocol/RubySlicc_Exports.sm 956ac83b0a58 
>   src/mem/protocol/RubySlicc_MemControl.sm 956ac83b0a58 
>   src/mem/protocol/RubySlicc_Types.sm 956ac83b0a58 
>   src/mem/ruby/SConsopts 956ac83b0a58 
>   src/mem/ruby/buffers/MessageBuffer.cc 956ac83b0a58 
>   src/mem/ruby/common/Address.hh 956ac83b0a58 
>   src/mem/ruby/common/DataBlock.hh 956ac83b0a58 
>   src/mem/ruby/common/Debug.hh 956ac83b0a58 
>   src/mem/ruby/common/Debug.cc 956ac83b0a58 
>   src/mem/ruby/common/NetDest.hh 956ac83b0a58 
>   src/mem/ruby/common/NetDest.cc 956ac83b0a58 
>   src/mem/ruby/network/garnet/fixed-pipeline/NetworkInterface_d.cc 
> 956ac83b0a58 
>   src/mem/ruby/network/garnet/fixed-pipeline/Switch_d.cc 956ac83b0a58 
>   src/mem/ruby/network/garnet/flexible-pipeline/NetworkInterface.cc 
> 956ac83b0a58 
>   src/mem/ruby/network/garnet/flexible-pipeline/Router.cc 956ac83b0a58 
>   src/mem/ruby/network/simple/PerfectSwitch.cc 956ac83b0a58 
>   src/mem/ruby/network/simple/Throttle.cc 956ac83b0a58 
>   src/mem/ruby/network/simple/Topology.cc 956ac83b0a58 
>   src/mem/ruby/slicc_interface/Message.hh 956ac83b0a58 
>   src/mem/ruby/slicc_interface/NetworkMessage.hh 956ac83b0a58 
>   src/mem/ruby/storebuffer/storebuffer.cc 956ac83b0a58 
>   src/mem/ruby/system/CacheMemory.cc 956ac83b0a58 
>   src/mem/ruby/system/DirectoryMemory.cc 956ac83b0a58 
>   src/mem/ruby/system/SConscript 956ac83b0a58 
>   src/mem/ruby/system/SparseMemory.cc 956ac83b0a58 
>   src/mem/ruby/tester/RaceyPseudoThread.cc 956ac83b0a58 
>   src/mem/slicc/ast/FuncCallExprAST.py 956ac83b0a58 
>   src/mem/slicc/symbols/StateMachine.py 956ac83b0a58 
>   src/mem/slicc/sym

Re: [m5-dev] Review Request: Moving Ruby to M5's debug print support

[Brad Beckmann]

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


I think Steve covered most of the main points.  Please see my detailed comment 
below about when I think line numbers and function names are useful.

Brad


src/mem/protocol/RubySlicc_MemControl.sm


I'll beat Nate to this.  :)  Try to avoid inserting and deleting random 
lines in files.  It cluters the diff and unecessarily makes merging harder.



src/mem/ruby/common/DataBlock.hh


Spacing is off.  Are you using mistakingly using tabs?



src/mem/ruby/common/Debug.hh


I agree with Steve's point that adding "__PRETTY_FUNCTION__, __FILE__, 
__LINE__" to every Ruby DPRINTF call is too cumbersome and we either need to 
remove it or define a macro that does it automatically.  Nilay, I think I might 
have confused you during our previous email conversations.  We definitely want 
to automatically include the function name, file, and line number for the 
SLICC/.sm debug statements...I've found that feature very useful in the past.  
However, I'm not sure we need it for the debug statements on the Ruby side.  
Nilay, if I understand your changes to SLICC correctly and recall from our 
previous conversations on this topic, these old ruby DEBUG macros relied on the 
ostream operator<< overloading, correct? That is why you now have the 
conditional statements in the SLICC that generate the correct DPRINTF line 
depending on the type.  Am I understanding the issues correctly?  If so, I 
think your solution for the SLICC generation of DPRINTF statements looks g
 ood.  I just think that we need to clean up the Ruby DPRINTF statements.




src/mem/ruby/common/NetDest.hh


Spacing again.



src/mem/slicc/ast/FuncCallExprAST.py


See long comment above.


- Brad


On 2010-10-19 17:30:48, Nilay Vaish wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/277/
> ---
> 
> (Updated 2010-10-19 17:30:48)
> 
> 
> 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 956ac83b0a58 
>   src/mem/SConscript 956ac83b0a58 
>   src/mem/protocol/MESI_CMP_directory-L1cache.sm 956ac83b0a58 
>   src/mem/protocol/MESI_CMP_directory-L2cache.sm 956ac83b0a58 
>   src/mem/protocol/MESI_CMP_directory-dir.sm 956ac83b0a58 
>   src/mem/protocol/MI_example-cache.sm 956ac83b0a58 
>   src/mem/protocol/MI_example-dir.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_directory-L1cache.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_directory-L2cache.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_directory-dir.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_directory-perfectDir.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_token-L1cache.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_token-L2cache.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_token-dir.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_hammer-cache.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_hammer-dir.sm 956ac83b0a58 
>   src/mem/protocol/RubySlicc_Exports.sm 956ac83b0a58 
>   src/mem/protocol/RubySlicc_MemControl.sm 956ac83b0a58 
>   src/mem/protocol/RubySlicc_Types.sm 956ac83b0a58 
>   src/mem/ruby/SConsopts 956ac83b0a58 
>   src/mem/ruby/buffers/MessageBuffer.cc 956ac83b0a58 
>   src/mem/ruby/common/Address.hh 956ac83b0a58 
>   src/mem/ruby/common/DataBlock.hh 956ac83b0a58 
>   src/mem/ruby/common/Debug.hh 956ac83b0a58 
>   src/mem/ruby/common/Debug.cc 956ac83b0a58 
>   src/mem/ruby/common/NetDest.hh 956ac83b0a58 
>   src/mem/ruby/common/NetDest.cc 956ac83b0a58 
>   src/mem/ruby/network/garnet/fixed-pipeline/NetworkInterface_d.cc 
> 956ac83b0a58 
>   src/mem/ruby/network/garnet/fixed-pipeline/Switch_d.cc 956ac83b0a58 
>   src/mem/ruby/network/garnet/flexible-pipeline/NetworkInterface.cc 
> 956ac83b0a58 
>   src/mem/ruby/network/garnet/flexible-pipeline/Router.cc 956ac83b0a58 
>   src/mem/ruby/network/simple/PerfectSwitch.cc 956ac83b0a58 
>   src/mem/ruby/network/simple/Throttle.cc 956ac83b0a58 
>   src/mem/ruby/network/simple/Topology.cc 956ac83b0a58 
>   src/mem/ruby/slicc_interface/Message.hh 956ac83b0a58 
>   src/mem/rub

Re: [m5-dev] Review Request: Moving Ruby to M5's debug print support

[Steve Reinhardt]

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



src/cpu/testers/rubytest/CheckTable.cc


These adjacent DPRINTFs should be consolidated into one.  It's more 
efficient (since it's a single check of the trace flag and a single call to 
cprintf), and in this case (and many others) we don't really need the function 
name, file name, and line number printed redundantly.

Also you don't need to use c_str(), cprintf() handles std::string just fine 
(and anything else that supports the '<<' operator, I believe).



src/mem/SConscript


Do we want to define a whole new disjoint set of trace flags just for Ruby? 
 For example, why have RubyCache and Cache both?



src/mem/protocol/MESI_CMP_directory-L1cache.sm


This DPRINTF is broken (and several others like it)... it needs a trace 
flag as the first arg, and "%str" probably isn't the format string you want 
(basically it's just like printf, so you probably mean "%s").



src/mem/protocol/MESI_CMP_directory-L1cache.sm


We should get rid of these commented-out lines, either deleting them if 
they're not needed or just using a trace flag that keeps them turned off unless 
wanted.  That's the point of the trace flags, is so that you don't have to 
comment out debug printfs, but can just turn them on and off dynamically at the 
granularity you want.



src/mem/ruby/buffers/MessageBuffer.cc


Note that DPRINTF automatically prepends the value of name(), so we should 
make sure that works for Ruby so we don't need to explicitly pass 
m_name.c_str() (which I assume is the same or similar).



src/mem/ruby/common/Address.hh


Indentation looks funky on this line.  Are you using tabs?



src/mem/ruby/common/Address.hh


Nate can correct me if I'm wrong, but the cprintf library eventually uses 
'<<' at the bottom, so if we need to define a type-specific output (here and 
with DataBlock below) we should just overload that operator.  In this specific 
case, much of this formatting can be handled in the format string itself as 
"[0x%x, line 0x%x]" and then calling with addr and blockAlign(addr) as 
arguments.




src/mem/ruby/common/Debug.hh


We need to decide how valuable it is to automatically include the function 
name, file, and line number.  If there's a consensus that we should keep it, we 
should add DPRINTF variants to M5 that have this.  If not, we should eliminate 
this info from the Ruby debug messages.  But we shouldn't take a bunch of 
boilerplate that used to be nicely encapsulated in a macro and reproduce it N 
times all over the code.



src/mem/ruby/common/Debug.cc


Code should never just be commented out... if we don't need it any more, 
just delete it.


- Steve


On 2010-10-19 17:30:48, Nilay Vaish wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/277/
> ---
> 
> (Updated 2010-10-19 17:30:48)
> 
> 
> 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 956ac83b0a58 
>   src/mem/SConscript 956ac83b0a58 
>   src/mem/protocol/MESI_CMP_directory-L1cache.sm 956ac83b0a58 
>   src/mem/protocol/MESI_CMP_directory-L2cache.sm 956ac83b0a58 
>   src/mem/protocol/MESI_CMP_directory-dir.sm 956ac83b0a58 
>   src/mem/protocol/MI_example-cache.sm 956ac83b0a58 
>   src/mem/protocol/MI_example-dir.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_directory-L1cache.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_directory-L2cache.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_directory-dir.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_directory-perfectDir.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_token-L1cache.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_token-L2cache.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_CMP_token-dir.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_hammer-cache.sm 956ac83b0a58 
>   src/mem/protocol/MOESI_hammer-dir.sm 956ac8

Re: [m5-dev] Review Request: Moving Ruby to M5's debug print support

[Nilay Vaish]

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

(Updated 2010-10-19 17:30:48.985205)


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 956ac83b0a58 
  src/mem/SConscript 956ac83b0a58 
  src/mem/protocol/MESI_CMP_directory-L1cache.sm 956ac83b0a58 
  src/mem/protocol/MESI_CMP_directory-L2cache.sm 956ac83b0a58 
  src/mem/protocol/MESI_CMP_directory-dir.sm 956ac83b0a58 
  src/mem/protocol/MI_example-cache.sm 956ac83b0a58 
  src/mem/protocol/MI_example-dir.sm 956ac83b0a58 
  src/mem/protocol/MOESI_CMP_directory-L1cache.sm 956ac83b0a58 
  src/mem/protocol/MOESI_CMP_directory-L2cache.sm 956ac83b0a58 
  src/mem/protocol/MOESI_CMP_directory-dir.sm 956ac83b0a58 
  src/mem/protocol/MOESI_CMP_directory-perfectDir.sm 956ac83b0a58 
  src/mem/protocol/MOESI_CMP_token-L1cache.sm 956ac83b0a58 
  src/mem/protocol/MOESI_CMP_token-L2cache.sm 956ac83b0a58 
  src/mem/protocol/MOESI_CMP_token-dir.sm 956ac83b0a58 
  src/mem/protocol/MOESI_hammer-cache.sm 956ac83b0a58 
  src/mem/protocol/MOESI_hammer-dir.sm 956ac83b0a58 
  src/mem/protocol/RubySlicc_Exports.sm 956ac83b0a58 
  src/mem/protocol/RubySlicc_MemControl.sm 956ac83b0a58 
  src/mem/protocol/RubySlicc_Types.sm 956ac83b0a58 
  src/mem/ruby/SConsopts 956ac83b0a58 
  src/mem/ruby/buffers/MessageBuffer.cc 956ac83b0a58 
  src/mem/ruby/common/Address.hh 956ac83b0a58 
  src/mem/ruby/common/DataBlock.hh 956ac83b0a58 
  src/mem/ruby/common/Debug.hh 956ac83b0a58 
  src/mem/ruby/common/Debug.cc 956ac83b0a58 
  src/mem/ruby/common/NetDest.hh 956ac83b0a58 
  src/mem/ruby/common/NetDest.cc 956ac83b0a58 
  src/mem/ruby/network/garnet/fixed-pipeline/NetworkInterface_d.cc 956ac83b0a58 
  src/mem/ruby/network/garnet/fixed-pipeline/Switch_d.cc 956ac83b0a58 
  src/mem/ruby/network/garnet/flexible-pipeline/NetworkInterface.cc 
956ac83b0a58 
  src/mem/ruby/network/garnet/flexible-pipeline/Router.cc 956ac83b0a58 
  src/mem/ruby/network/simple/PerfectSwitch.cc 956ac83b0a58 
  src/mem/ruby/network/simple/Throttle.cc 956ac83b0a58 
  src/mem/ruby/network/simple/Topology.cc 956ac83b0a58 
  src/mem/ruby/slicc_interface/Message.hh 956ac83b0a58 
  src/mem/ruby/slicc_interface/NetworkMessage.hh 956ac83b0a58 
  src/mem/ruby/storebuffer/storebuffer.cc 956ac83b0a58 
  src/mem/ruby/system/CacheMemory.cc 956ac83b0a58 
  src/mem/ruby/system/DirectoryMemory.cc 956ac83b0a58 
  src/mem/ruby/system/SConscript 956ac83b0a58 
  src/mem/ruby/system/SparseMemory.cc 956ac83b0a58 
  src/mem/ruby/tester/RaceyPseudoThread.cc 956ac83b0a58 
  src/mem/slicc/ast/FuncCallExprAST.py 956ac83b0a58 
  src/mem/slicc/symbols/StateMachine.py 956ac83b0a58 
  src/mem/slicc/symbols/Type.py 956ac83b0a58 

Diff: http://reviews.m5sim.org/r/277/diff


Testing
---


Thanks,

Nilay

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

[m5-dev] Review Request: Moving Ruby to M5's debug print support

[Nilay Vaish]

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

Review request for 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 956ac83b0a58 
  src/mem/SConscript 956ac83b0a58 
  src/mem/protocol/MESI_CMP_directory-L1cache.sm 956ac83b0a58 
  src/mem/protocol/MESI_CMP_directory-L2cache.sm 956ac83b0a58 
  src/mem/protocol/MESI_CMP_directory-dir.sm 956ac83b0a58 
  src/mem/protocol/MI_example-cache.sm 956ac83b0a58 
  src/mem/protocol/MI_example-dir.sm 956ac83b0a58 
  src/mem/protocol/MOESI_CMP_directory-L1cache.sm 956ac83b0a58 
  src/mem/protocol/MOESI_CMP_directory-L2cache.sm 956ac83b0a58 
  src/mem/protocol/MOESI_CMP_directory-dir.sm 956ac83b0a58 
  src/mem/protocol/MOESI_CMP_directory-perfectDir.sm 956ac83b0a58 
  src/mem/protocol/MOESI_CMP_token-L1cache.sm 956ac83b0a58 
  src/mem/protocol/MOESI_CMP_token-L2cache.sm 956ac83b0a58 
  src/mem/protocol/MOESI_CMP_token-dir.sm 956ac83b0a58 
  src/mem/protocol/MOESI_hammer-cache.sm 956ac83b0a58 
  src/mem/protocol/MOESI_hammer-dir.sm 956ac83b0a58 
  src/mem/protocol/RubySlicc_Exports.sm 956ac83b0a58 
  src/mem/protocol/RubySlicc_MemControl.sm 956ac83b0a58 
  src/mem/protocol/RubySlicc_Types.sm 956ac83b0a58 
  src/mem/ruby/SConsopts 956ac83b0a58 
  src/mem/ruby/buffers/MessageBuffer.cc 956ac83b0a58 
  src/mem/ruby/common/Address.hh 956ac83b0a58 
  src/mem/ruby/common/DataBlock.hh 956ac83b0a58 
  src/mem/ruby/common/Debug.hh 956ac83b0a58 
  src/mem/ruby/common/Debug.cc 956ac83b0a58 
  src/mem/ruby/common/NetDest.hh 956ac83b0a58 
  src/mem/ruby/common/NetDest.cc 956ac83b0a58 
  src/mem/ruby/network/garnet/fixed-pipeline/NetworkInterface_d.cc 956ac83b0a58 
  src/mem/ruby/network/garnet/fixed-pipeline/Switch_d.cc 956ac83b0a58 
  src/mem/ruby/network/garnet/flexible-pipeline/NetworkInterface.cc 
956ac83b0a58 
  src/mem/ruby/network/garnet/flexible-pipeline/Router.cc 956ac83b0a58 
  src/mem/ruby/network/simple/PerfectSwitch.cc 956ac83b0a58 
  src/mem/ruby/network/simple/Throttle.cc 956ac83b0a58 
  src/mem/ruby/network/simple/Topology.cc 956ac83b0a58 
  src/mem/ruby/slicc_interface/Message.hh 956ac83b0a58 
  src/mem/ruby/slicc_interface/NetworkMessage.hh 956ac83b0a58 
  src/mem/ruby/storebuffer/storebuffer.cc 956ac83b0a58 
  src/mem/ruby/system/CacheMemory.cc 956ac83b0a58 
  src/mem/ruby/system/DirectoryMemory.cc 956ac83b0a58 
  src/mem/ruby/system/SConscript 956ac83b0a58 
  src/mem/ruby/system/SparseMemory.cc 956ac83b0a58 
  src/mem/ruby/tester/RaceyPseudoThread.cc 956ac83b0a58 
  src/mem/slicc/ast/FuncCallExprAST.py 956ac83b0a58 
  src/mem/slicc/symbols/StateMachine.py 956ac83b0a58 
  src/mem/slicc/symbols/Type.py 956ac83b0a58 

Diff: http://reviews.m5sim.org/r/277/diff


Testing
---


Thanks,

Nilay

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