date:20110104



 On 2011-01-03 15:31:20, Brad Beckmann wrote:
  Hi Nilay,

First, I must say this is an impressive amount of work.  You definitely got a 
lot done over holiday break. :)

Overall, this set of patches is definitely close, but I want to see if we can 
take them a step forward.  Also I have a few suggestions that may make things 
easier.  Finally, I have a bunch of minor questions/suggestions on individual 
lines, but I’ll hold off on those until you can respond to my higher-level 
questions.

The main thing I would like to see improved is not having to differentiate 
between “entry” and “entry_ptr” in the .sm files.  Am I correct that the only 
functions in the .sm files that are passed an “entry_ptr” are “is_valid_ptr”, 
“getCacheEntry”, and “set_cache_entry”?  If so, it seems that all three 
functions are generated with unique python code, either in an AST file or 
StateMachine.py.  Therefore, could we just pass these functions “entry” and 
rely on the underneath python code to generate the correct references?  This 
would make things more readable, “is_valid_ptr()” becomes “is_valid”, and it 
doesn’t require the slicc programmer to understand which functions take an 
entry pointer versus the entry itself.  If we can’t make such a change, I worry 
about how much extra complexity this change pushes on the slicc programmer.

Also another suggestion to make things more readable, please replace the name 
L1IcacheMemory_entry with L1I_entry.  Do the same for L1D_entry and L2_entry.  
That will shorten many of your lines.

So am I correct that hammer’s simultaneous usage of valid L1 and L2 cache 
entries in certain transitions is the only reason that within all actions, the 
getCacheEntry calls take multiple cache entries?  If so, I think it would be 
fairly trivial to use a tbe entry as an intermediary between the L1 and L2 for 
those particular hammer transitions.  That way only one cache entry is valid at 
any particular time, and we can simply use the variable cache_entry in the 
actions.  That should clean things up a lot.

By the way, once you check in these patches, the MESI_CMP_directory protocol 
will be deprecated, correct?  If so, make sure you include a patch that removes 
it from the regression tester.

Brad

 The main thing I would like to see improved is not having to differentiate
 between â€œentryâ€ and â€œentry_ptrâ€ in the .sm files.  Am I correct
 that the only functions in the .sm files that are passed an
 â€œentry_ptrâ€ are â€œis_valid_ptrâ€, â€œgetCacheEntryâ€, and
 â€œset_cache_entryâ€?  If so, it seems that all three functions are
 generated with unique python code, either in an AST file or
 StateMachine.py.  Therefore, could we just pass these functions
 â€œentryâ€ and rely on the underneath python code to generate the correct
 references?  This would make things more readable, â€œis_valid_ptr()â€
 becomes â€œis_validâ€, and it doesnâ€™t require the slicc programmer to
 understand which functions take an entry pointer versus the entry itself. 
 If we canâ€™t make such a change, I worry about how much extra complexity
 this change pushes on the slicc programmer.

There are functions that are passed cache entry and transaction buffer entry as 
arguments. Currently, I assume that these arguments are passed using pointers.

 
 Also another suggestion to make things more readable, please replace the
 name L1IcacheMemory_entry with L1I_entry.  Do the same for L1D_entry and
 L2_entry.  That will shorten many of your lines.

The names of the cache entry variables are currently tied with the names of the 
cache memory variables belonging to the machine. If the name of the cache 
memory variable is A, then the corresponding cache entry variable is named 
A_entry.

 So am I correct that hammerâ€™s simultaneous usage of valid L1 and L2
 cache entries in certain transitions is the only reason that within all
 actions, the getCacheEntry calls take multiple cache entries?  If so, I
 think it would be fairly trivial to use a tbe entry as an intermediary
 between the L1 and L2 for those particular hammer transitions.  That way
 only one cache entry is valid at any particular time, and we can simply
 use the variable cache_entry in the actions.  That should clean things up
 a lot.

Oops! Should have thought of that before doing all those changes. But can we 
assume that we would always have only one valid cache entry pointer at any 
given time? If that's true, I would probably revert to previous version of the 
patch. This should also resolve the naming issue.

 By the way, once you check in these patches, the MESI_CMP_directory
 protocol will be deprecated, correct?  If so, make sure you include a
 patch that removes it from the regression tester.

I have a patch for the protocol, but I need to discuss it. Do you think it is 
possible that a protocol is not in a dead lock but random tester outputs so 
because the underlying memory system is taking too much time? The patch works 
for 1, 2,

Re: [m5-dev] Review Request: Changing how CacheMemory interfaces with SLICC

2011-01-04 Thread Arkaprava Basu


Hi Nilay,

   On deadlock issue with MESI_CMP_directory :
   Yes,  this can happen as ruby_tester or Sequencer only reports 
*possible* deadlocks. With higher number of processors there is more 
contention (and thus latency) and it can mistakenly report deadlock. I 
generally look at the protocol trace to figure out whether there is 
actually any deadlock or not. You can also try doubling the Sequencer 
deadlock threshold and see if the problem goes away. If its a true 
deadlock, it will  break again.


On some related note,  as Brad has pointed out MESI_CMP_directory has 
its share of issues. Recently one of Prof. Sarita Adve's student 
e-mailed us (Multifacet) about 6 bugs he found while model checking the 
MESI_CMP_directory (including a major one). I took some time to look at 
them and it seems like MESI_CMP_directory is now fixed (hopefully).  The 
modified protocol is now passing 1M checks with 16 processors with 
multiple random seeds.  I can locally coordinate with you on this, if 
you want.


Thanks
Arka

On 01/04/2011 11:43 AM, Nilay Vaish wrote:



On 2011-01-03 15:31:20, Brad Beckmann wrote:

Hi Nilay,

First, I must say this is an impressive amount of work.  You definitely got a 
lot done over holiday break. :)

Overall, this set of patches is definitely close, but I want to see if we can 
take them a step forward.  Also I have a few suggestions that may make things 
easier.  Finally, I have a bunch of minor questions/suggestions on individual 
lines, but I’ll hold off on those until you can respond to my higher-level 
questions.

The main thing I would like to see improved is not having to differentiate 
between “entry” and “entry_ptr” in the .sm files.  Am I correct that the only 
functions in the .sm files that are passed an “entry_ptr” are “is_valid_ptr”, 
“getCacheEntry”, and “set_cache_entry”?  If so, it seems that all three 
functions are generated with unique python code, either in an AST file or 
StateMachine.py.  Therefore, could we just pass these functions “entry” and 
rely on the underneath python code to generate the correct references?  This 
would make things more readable, “is_valid_ptr()” becomes “is_valid”, and it 
doesn’t require the slicc programmer to understand which functions take an 
entry pointer versus the entry itself.  If we can’t make such a change, I worry 
about how much extra complexity this change pushes on the slicc programmer.

Also another suggestion to make things more readable, please replace the name 
L1IcacheMemory_entry with L1I_entry.  Do the same for L1D_entry and L2_entry.  
That will shorten many of your lines.

So am I correct that hammer’s simultaneous usage of valid L1 and L2 cache 
entries in certain transitions is the only reason that within all actions, the 
getCacheEntry calls take multiple cache entries?  If so, I think it would be 
fairly trivial to use a tbe entry as an intermediary between the L1 and L2 for 
those particular hammer transitions.  That way only one cache entry is valid at 
any particular time, and we can simply use the variable cache_entry in the 
actions.  That should clean things up a lot.

By the way, once you check in these patches, the MESI_CMP_directory protocol 
will be deprecated, correct?  If so, make sure you include a patch that removes 
it from the regression tester.

Brad


The main thing I would like to see improved is not having to differentiate
between â€œentryâ€ and â€œentry_ptrâ€ in the .sm files.  Am I correct
that the only functions in the .sm files that are passed an
â€œentry_ptrâ€ are â€œis_valid_ptrâ€, â€œgetCacheEntryâ€, and
â€œset_cache_entryâ€?  If so, it seems that all three functions are
generated with unique python code, either in an AST file or
StateMachine.py.  Therefore, could we just pass these functions
â€œentryâ€ and rely on the underneath python code to generate the correct
references?  This would make things more readable, â€œis_valid_ptr()â€
becomes â€œis_validâ€, and it doesnâ€™t require the slicc programmer to
understand which functions take an entry pointer versus the entry itself.
If we canâ€™t make such a change, I worry about how much extra complexity
this change pushes on the slicc programmer.

There are functions that are passed cache entry and transaction buffer entry as 
arguments. Currently, I assume that these arguments are passed using pointers.


Also another suggestion to make things more readable, please replace the
name L1IcacheMemory_entry with L1I_entry.  Do the same for L1D_entry and
L2_entry.  That will shorten many of your lines.

The names of the cache entry variables are currently tied with the names of the 
cache memory variables belonging to the machine. If the name of the cache 
memory variable is A, then the corresponding cache entry variable is named 
A_entry.


So am I correct that hammerâ€™s simultaneous usage of valid L1 and L2
cache entries in certain transitions is the only reason that within all
actions, the getCacheEntry calls take

Re: [m5-dev] Review Request: Changing how CacheMemory interfaces with SLICC

2011-01-04 Thread Beckmann, Brad

Hi Nilay,

My responses are below:

 The main thing I would like to see improved is not having to differentiate
 between â€œentryâ€ and â€œentry_ptrâ€ in the .sm files.  Am I correct
 that the only functions in the .sm files that are passed an
 â€œentry_ptrâ€ are â€œis_valid_ptrâ€, â€œgetCacheEntryâ€, and
 â€œset_cache_entryâ€?  If so, it seems that all three functions are
 generated with unique python code, either in an AST file or
 StateMachine.py.  Therefore, could we just pass these functions
 â€œentryâ€ and rely on the underneath python code to generate the correct
 references?  This would make things more readable, â€œis_valid_ptr()â€
 becomes â€œis_validâ€, and it doesnâ€™t require the slicc programmer to
 understand which functions take an entry pointer versus the entry itself. 
 If we canâ€™t make such a change, I worry about how much extra complexity
 this change pushes on the slicc programmer.

There are functions that are passed cache entry and transaction buffer entry as 
arguments. Currently, I assume that these arguments are passed using pointers.

[BB] So does that mean that the cache entry is always passed in as a pointer?  
If so, can one just use cache_entry for all function calls and remove any use 
of cache_entry_ptr in the .sm files?  That is essentially what I would like 
to see.  

 
 Also another suggestion to make things more readable, please replace the
 name L1IcacheMemory_entry with L1I_entry.  Do the same for L1D_entry and
 L2_entry.  That will shorten many of your lines.

The names of the cache entry variables are currently tied with the names of the 
cache memory variables belonging to the machine. If the name of the cache 
memory variable is A, then the corresponding cache entry variable is named 
A_entry.

[BB] Ah, I see.  Ok then let's just keep them the way they are for now.  We can 
deal with shorting the names later.

 So am I correct that hammerâ€™s simultaneous usage of valid L1 and L2
 cache entries in certain transitions is the only reason that within all
 actions, the getCacheEntry calls take multiple cache entries?  If so, I
 think it would be fairly trivial to use a tbe entry as an intermediary
 between the L1 and L2 for those particular hammer transitions.  That way
 only one cache entry is valid at any particular time, and we can simply
 use the variable cache_entry in the actions.  That should clean things up
 a lot.

Oops! Should have thought of that before doing all those changes. But can we 
assume that we would always have only one valid cache entry pointer at any 
given time? If that's true, I would probably revert to previous version of the 
patch. This should also resolve the naming issue.

[BB] I wouldn't have expected you to realize that.  It is one of those things 
that isn't completely obvious without spending a lot of time developing 
protocols.  Yes, I think it is easiest for you to just revert to the previous 
version of the patch and just modify the hammer protocol to use a tbe entry as 
an intermediary.  We've always had an unofficial rule that a controller can 
only manage multiple caches if those caches are exclusive with respect to each 
other.  For the most part, that rule has been followed by all the protocols I'm 
familiar with.  I think your change just makes that an official policy.

 By the way, once you check in these patches, the MESI_CMP_directory
 protocol will be deprecated, correct?  If so, make sure you include a
 patch that removes it from the regression tester.

I have a patch for the protocol, but I need to discuss it. Do you think it is 
possible that a protocol is not in a dead lock but random tester outputs so 
because the underlying memory system is taking too much time? The patch works 
for 1, 2, and 4 processors for 10,000,000 loads. I have tested these processor 
configurations with 40 different seed values. But for 8 processors, random 
tester outputs some thing like this --

panic: Possible Deadlock detected. Aborting!
version: 6 request.paddr: 12779 m_writeRequestTable: 15 current time: 369500011 
issue_time: 368993771 difference: 506240
 @ cycle 369500011
[wakeup:build/ALPHA_SE_MESI_CMP_directory/mem/ruby/system/Sequencer.cc, line 
123]

[BB] Yes, the current version of MESI_CMP_directory is broken in many places.  
Arka just told me that he recently fixed many of those problems.  I suggest 
getting his fixes and working from there.

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

[m5-dev] Fixing MESI CMP directory protocol


What threshold do you use?

On Tue, 4 Jan 2011, Arkaprava Basu wrote:


Hi Nilay,

  On deadlock issue with MESI_CMP_directory :
  Yes,  this can happen as ruby_tester or Sequencer only reports *possible* 
deadlocks. With higher number of processors there is more contention (and 
thus latency) and it can mistakenly report deadlock. I generally look at the 
protocol trace to figure out whether there is actually any deadlock or not. 
You can also try doubling the Sequencer deadlock threshold and see if the 
problem goes away. If its a true deadlock, it will  break again.


On some related note,  as Brad has pointed out MESI_CMP_directory has its 
share of issues. Recently one of Prof. Sarita Adve's student e-mailed us 
(Multifacet) about 6 bugs he found while model checking the 
MESI_CMP_directory (including a major one). I took some time to look at them 
and it seems like MESI_CMP_directory is now fixed (hopefully).  The modified 
protocol is now passing 1M checks with 16 processors with multiple random 
seeds.  I can locally coordinate with you on this, if you want.


Thanks
Arka


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

Re: [m5-dev] Review Request: scons: show sources and targets when building.

On Mon, Jan 3, 2011 at 10:41 PM, nathan binkert n...@binkert.org wrote:

  So far I've found two oddities with a straightforward implementation of
 this
  approach:
 
  (1) It breaks down when the target is a prefix of the source:
 
   [   STRIP] ALPHA_SE/m5.fast.unstripped -
 
  instead of
 
   [   STRIP] ALPHA_SE/m5.fast: .unstripped -
 
  (2) It gives the wrong impression when targets are in a parent directory
  relative to the source:
 
   [ISA DESC] ALPHA_SE/arch/alpha/isa/main.isa - decoder.cc, decoder.hh,
  max_inst_regs.hh, atomic_simple_cpu_exec.cc, inorder_cpu_exec.cc,
  o3_cpu_exec.cc, timing_simple_cpu_exec.cc
 
  instead of
 
   [ISA DESC] ALPHA_SE/arch/alpha: isa/main.isa - decoder.cc, decoder.hh,
  max_inst_regs.hh, atomic_simple_cpu_exec.cc, inorder_cpu_exec.cc,
  o3_cpu_exec.cc, timing_simple_cpu_exec.cc
 
  I don't see these as deal-breakers, and I'm fine with leaving them as
 they
  are unless someone has a reasonable idea about how to address them, but I
  thought I'd point out that the ambiguities aren't purely theoretical.

 hmmm  I guess I still like the ability to cut and paste, but the
 ambiguity is annoying (and there are bound to be others.  I'll let you
 decide.  For #2, you could do ../decoder.cc, for #1, if you want, you
 could detect that the RHS is empty and do the whole basename.  These
 ideas might be overkill.  (Don't you love how a simple idea always
 gets more complicated?)


Yea, given the amount of time I've spent on this already I'm not inclined to
add a lot of sophisticated handling for a few exceptional cases.  #2 in
particular doesn't seem that bad to me; it is ambiguous, but in a sort of
reasonable way, and adding a bunch of path-handling code seems like
overkill.  For #1, the output is less intuitive with the blank RHS, and it's
pretty easy to check that the target is the same length as the common prefix
to know that there's a problem.  The question is what to do about it... one
simple possibility is just to reverse the arrow:

 [   STRIP] ALPHA_SE/m5.fast - .unstripped

Or I could trim back to the path separator:

 [   STRIP] ALPHA_SE/m5.fast.unstripped - m5.fast

Or trim to the nearest '.' or path sep:

 [   STRIP] ALPHA_SE/m5.fast.unstripped - .fast

Any preferences?

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

Re: [m5-dev] Review Request: scons: show sources and targets when building.

2011-01-04 Thread nathan binkert

  [   STRIP] ALPHA_SE/m5.fast - .unstripped
Would it be for just this one, or all? I think that we generally want
to cut and paste the source, not the target, so either way, this
doesn't seem like a great option.

 Or I could trim back to the path separator:
  [   STRIP] ALPHA_SE/m5.fast.unstripped - m5.fast

 Or trim to the nearest '.' or path sep:
  [   STRIP] ALPHA_SE/m5.fast.unstripped - .fast
Again, would this apply to all?  if so, I'd probably opt for the last
one since we don't need it in the majority of cases.

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

Re: [m5-dev] Fixing MESI CMP directory protocol

2011-01-04 Thread Arkaprava Basu


These are the following step I use:

1. First run with whatever default values of threshold are.
2. If deadlocked, take trace and try to find out is there evident reason 
for deadlock or not.

3. If no, double the default threshold value and run again.
4. If the same test passes with larger threshold, then it means the 
deadlock was actually not there. So life is good. If not, need to dig 
more into trace to see whats going on.


@Nilay:
By end of today, I will share with you the patch that seems like fixed  
that protocol.


Thanks
Arka

On 01/04/2011 12:51 PM, Nilay Vaish wrote:

What threshold do you use?

On Tue, 4 Jan 2011, Arkaprava Basu wrote:


Hi Nilay,

  On deadlock issue with MESI_CMP_directory :
  Yes,  this can happen as ruby_tester or Sequencer only reports 
*possible* deadlocks. With higher number of processors there is more 
contention (and thus latency) and it can mistakenly report deadlock. 
I generally look at the protocol trace to figure out whether there is 
actually any deadlock or not. You can also try doubling the Sequencer 
deadlock threshold and see if the problem goes away. If its a true 
deadlock, it will  break again.


On some related note,  as Brad has pointed out MESI_CMP_directory has 
its share of issues. Recently one of Prof. Sarita Adve's student 
e-mailed us (Multifacet) about 6 bugs he found while model checking 
the MESI_CMP_directory (including a major one). I took some time to 
look at them and it seems like MESI_CMP_directory is now fixed 
(hopefully).  The modified protocol is now passing 1M checks with 16 
processors with multiple random seeds.  I can locally coordinate with 
you on this, if you want.


Thanks
Arka


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

Re: [m5-dev] Review Request: scons: show sources and targets when building.

On Tue, Jan 4, 2011 at 11:24 AM, nathan binkert n...@binkert.org wrote:

   [   STRIP] ALPHA_SE/m5.fast - .unstripped
 Would it be for just this one, or all? I think that we generally want
 to cut and paste the source, not the target, so either way, this
 doesn't seem like a great option.


For all of these, I was thinking just for this case (or strictly, just for
the ones where the common prefix is equal to the entire target, which leaves
the RHS empty with the current code).

If you always reverse the arrow, you'll end up having the same problem in
the other direction (e.g., for the python embed step where foo.py -
foo.py.cc).


   Or I could trim back to the path separator:
   [   STRIP] ALPHA_SE/m5.fast.unstripped - m5.fast
 
  Or trim to the nearest '.' or path sep:
   [   STRIP] ALPHA_SE/m5.fast.unstripped - .fast
 Again, would this apply to all?  if so, I'd probably opt for the last
 one since we don't need it in the majority of cases.


 The current code works 99% of the time... always trimming back to the
nearest anything can cause issues; for example, with the swig lines:
bar/foo.i - _wrap.cc, .py
it looks nice as it is, but the common prefix is just bar/foo, so you don't
want to unconditionally go back to look for a . or path sep.

OTOH, I already back up a char when the common prefix ends in '.', otherwise
you'd get
foo.cc - o
instead of
foo.cc - .o
so maybe I can find a way to generalize that which will take care of the
.unstripped case.

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

Re: [m5-dev] Review Request: Changing how CacheMemory interfaces with SLICC

Brad

Is there a reason why each action name follows the pattern combination of
several letters_action performed by the action? The letters used are
not abbreviations of the action performed. Can we use any combination?

Thanks
Nilay

On Tue, 4 Jan 2011, Beckmann, Brad wrote:

Hi Nilay,

My responses are below:

The main thing I would like to see improved is not having to differentiate
between â€œentryâ€ and â€œentry_ptrâ€ in the .sm files. Am I correct
that the only functions in the .sm files that are passed an
â€œentry_ptrâ€ are â€œis_valid_ptrâ€, â€œgetCacheEntryâ€, and
â€œset_cache_entryâ€? If so, it seems that all three functions are
generated with unique python code, either in an AST file or
StateMachine.py. Therefore, could we just pass these functions
â€œentryâ€ and rely on the underneath python code to generate the correct
references? This would make things more readable, â€œis_valid_ptr()â€
becomes â€œis_validâ€, and it doesnâ€™t require the slicc programmer to
understand which functions take an entry pointer versus the entry itself.
If we canâ€™t make such a change, I worry about how much extra complexity
this change pushes on the slicc programmer.

There are functions that are passed cache entry and transaction buffer entry as
arguments. Currently, I assume that these arguments are passed using pointers.

[BB] So does that mean that the cache entry is always passed in as a pointer? If so, can one just
use cache_entry for all function calls and remove any use of
cache_entry_ptr in the .sm files? That is essentially what I would like to see.

Also another suggestion to make things more readable, please replace the
name L1IcacheMemory_entry with L1I_entry. Do the same for L1D_entry and
L2_entry. That will shorten many of your lines.

The names of the cache entry variables are currently tied with the names of the
cache memory variables belonging to the machine. If the name of the cache
memory variable is A, then the corresponding cache entry variable is named
A_entry.

[BB] Ah, I see. Ok then let's just keep them the way they are for now. We can
deal with shorting the names later.

So am I correct that hammerâ€™s simultaneous usage of valid L1 and L2
cache entries in certain transitions is the only reason that within all
actions, the getCacheEntry calls take multiple cache entries? If so, I
think it would be fairly trivial to use a tbe entry as an intermediary
between the L1 and L2 for those particular hammer transitions. That way
only one cache entry is valid at any particular time, and we can simply
use the variable cache_entry in the actions. That should clean things up
a lot.

Oops! Should have thought of that before doing all those changes. But can we
assume that we would always have only one valid cache entry pointer at any
given time? If that's true, I would probably revert to previous version of the
patch. This should also resolve the naming issue.

[BB] I wouldn't have expected you to realize that. It is one of those things
that isn't completely obvious without spending a lot of time developing
protocols. Yes, I think it is easiest for you to just revert to the previous
version of the patch and just modify the hammer protocol to use a tbe entry as
an intermediary. We've always had an unofficial rule that a controller can
only manage multiple caches if those caches are exclusive with respect to each
other. For the most part, that rule has been followed by all the protocols I'm
familiar with. I think your change just makes that an official policy.

By the way, once you check in these patches, the MESI_CMP_directory
protocol will be deprecated, correct? If so, make sure you include a
patch that removes it from the regression tester.

I have a patch for the protocol, but I need to discuss it. Do you think it is
possible that a protocol is not in a dead lock but random tester outputs so
because the underlying memory system is taking too much time? The patch works
for 1, 2, and 4 processors for 10,000,000 loads. I have tested these processor
configurations with 40 different seed values. But for 8 processors, random
tester outputs some thing like this --

panic: Possible Deadlock detected. Aborting!
version: 6 request.paddr: 12779 m_writeRequestTable: 15 current time: 369500011
issue_time: 368993771 difference: 506240
@ cycle 369500011
[wakeup:build/ALPHA_SE_MESI_CMP_directory/mem/ruby/system/Sequencer.cc, line
123]

[BB] Yes, the current version of MESI_CMP_directory is broken in many places.
Arka just told me that he recently fixed many of those problems. I suggest
getting his fixes and working from there.

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

Re: [m5-dev] Review Request: Changing how CacheMemory interfaces with SLICC

2011-01-04 Thread Beckmann, Brad

Hi Nilay,

At one point in time, the combination of several letters at the beginning of 
the action name corresponded to the short hand name for the action.  The short 
hand name is the letter or letter combination that appears in the HTML tables.  
SLICC may have once enforced that the combination of letters matched the HTML 
short hand name, but I don't believe it does right now.  Therefore the letters 
are just a guide to match actions with their associated short hand name.  And 
yes, you can use any combination.

Brad


-Original Message-
From: Nilay Vaish [mailto:ni...@cs.wisc.edu] 
Sent: Tuesday, January 04, 2011 12:03 PM
To: Beckmann, Brad
Cc: Default
Subject: RE: Review Request: Changing how CacheMemory interfaces with SLICC

Brad

Is there a reason why each action name follows the pattern combination of 
several letters_action performed by the action? The letters used are not 
abbreviations of the action performed. Can we use any combination?

Thanks
Nilay

On Tue, 4 Jan 2011, Beckmann, Brad wrote:

 Hi Nilay,

 My responses are below:

 The main thing I would like to see improved is not having to 
 differentiate between â€œentryâ€ and â€œentry_ptrâ€ in the .sm 
 files.  Am I correct that the only functions in the .sm files that 
 are passed an â€œentry_ptrâ€ are â€œis_valid_ptrâ€, 
 â€œgetCacheEntryâ€, and â€œset_cache_entryâ€?  If so, it seems that 
 all three functions are generated with unique python code, either in 
 an AST file or StateMachine.py.  Therefore, could we just pass these 
 functions â€œentryâ€ and rely on the underneath python code to 
 generate the correct references?  This would make things more 
 readable, â€œis_valid_ptr()â€ becomes â€œis_validâ€, and it 
 doesnâ€™t require the slicc programmer to understand which functions take an 
 entry pointer versus the entry itself.
 If we canâ€™t make such a change, I worry about how much extra 
 complexity this change pushes on the slicc programmer.

 There are functions that are passed cache entry and transaction buffer entry 
 as arguments. Currently, I assume that these arguments are passed using 
 pointers.

 [BB] So does that mean that the cache entry is always passed in as a pointer? 
  If so, can one just use cache_entry for all function calls and remove any 
 use of cache_entry_ptr in the .sm files?  That is essentially what I would 
 like to see.


 Also another suggestion to make things more readable, please replace 
 the name L1IcacheMemory_entry with L1I_entry.  Do the same for 
 L1D_entry and L2_entry.  That will shorten many of your lines.

 The names of the cache entry variables are currently tied with the names of 
 the cache memory variables belonging to the machine. If the name of the cache 
 memory variable is A, then the corresponding cache entry variable is named 
 A_entry.

 [BB] Ah, I see.  Ok then let's just keep them the way they are for now.  We 
 can deal with shorting the names later.

 So am I correct that hammerâ€™s simultaneous usage of valid L1 and L2 
 cache entries in certain transitions is the only reason that within 
 all actions, the getCacheEntry calls take multiple cache entries?  If 
 so, I think it would be fairly trivial to use a tbe entry as an 
 intermediary between the L1 and L2 for those particular hammer 
 transitions.  That way only one cache entry is valid at any 
 particular time, and we can simply use the variable cache_entry in 
 the actions.  That should clean things up a lot.

 Oops! Should have thought of that before doing all those changes. But can we 
 assume that we would always have only one valid cache entry pointer at any 
 given time? If that's true, I would probably revert to previous version of 
 the patch. This should also resolve the naming issue.

 [BB] I wouldn't have expected you to realize that.  It is one of those things 
 that isn't completely obvious without spending a lot of time developing 
 protocols.  Yes, I think it is easiest for you to just revert to the previous 
 version of the patch and just modify the hammer protocol to use a tbe entry 
 as an intermediary.  We've always had an unofficial rule that a controller 
 can only manage multiple caches if those caches are exclusive with respect to 
 each other.  For the most part, that rule has been followed by all the 
 protocols I'm familiar with.  I think your change just makes that an official 
 policy.

 By the way, once you check in these patches, the MESI_CMP_directory 
 protocol will be deprecated, correct?  If so, make sure you include a 
 patch that removes it from the regression tester.

 I have a patch for the protocol, but I need to discuss it. Do you 
 think it is possible that a protocol is not in a dead lock but random 
 tester outputs so because the underlying memory system is taking too 
 much time? The patch works for 1, 2, and 4 processors for 10,000,000 
 loads. I have tested these processor configurations with 40 different 
 seed values. But for 8 processors, random

[m5-dev] changeset in m5: Params: Print the IP components in the right or...

2011-01-04 Thread Gabe Black

changeset c819526b7c2a in /z/repo/m5
details: http://repo.m5sim.org/m5?cmd=changeset;node=c819526b7c2a
description:
Params: Print the IP components in the right order.

diffstat:

 src/base/inet.cc |  4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diffs (14 lines):

diff -r 7338bc628489 -r c819526b7c2a src/base/inet.cc
--- a/src/base/inet.cc  Mon Jan 03 14:35:47 2011 -0800
+++ b/src/base/inet.cc  Tue Jan 04 17:11:49 2011 -0500
@@ -138,8 +138,8 @@
 {
 uint32_t ip = ia.ip();
 ccprintf(stream, %x.%x.%x.%x,
-(uint8_t)(ip  0),  (uint8_t)(ip  8),
-(uint8_t)(ip  16), (uint8_t)(ip  24));
+(uint8_t)(ip  24), (uint8_t)(ip  16),
+(uint8_t)(ip  8),  (uint8_t)(ip  0));
 return stream;
 }
 
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

[m5-dev] Review Request: inorder: get rid of references to mainEventQueue.


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

Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan 
Binkert.


Summary
---

inorder: get rid of references to mainEventQueue.
Events need to be scheduled on the queue assigned
to the SimObject, not on the global queue (which
should be going away).
Also cleaned up a number of redundant expressions
that made the code unnecessarily verbose.


Diffs
-

  src/cpu/inorder/cpu.hh 7338bc628489 
  src/cpu/inorder/cpu.cc 7338bc628489 
  src/cpu/inorder/resource.hh 7338bc628489 
  src/cpu/inorder/resource.cc 7338bc628489 
  src/cpu/inorder/resource_pool.cc 7338bc628489 

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


Testing
---


Thanks,

Steve

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

[m5-dev] Review Request: inorder: replace schedEvent() code with reschedule().


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

Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan 
Binkert.


Summary
---

inorder: replace schedEvent() code with reschedule().
There were several copies of similar functions that looked
like they all replicated reschedule(), so I replaced them
with direct calls.  Keeping this separate from the previous
cset since there may be some subtle functional differences
if the code ever reschedules an event that is scheduled but
not squashed (though none were detected in the regressions).


Diffs
-

  src/cpu/inorder/cpu.hh 7338bc628489 
  src/cpu/inorder/cpu.cc 7338bc628489 
  src/cpu/inorder/resource.cc 7338bc628489 
  src/cpu/inorder/resource_pool.cc 7338bc628489 

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


Testing
---


Thanks,

Steve

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

[m5-dev] Review Request: pseudoinst: get rid of mainEventQueue references.


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

Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan 
Binkert.


Summary
---

pseudoinst: get rid of mainEventQueue references.
Avoid direct references to mainEventQueue in pseudo-insts
by indirecting through associated CPU object.
Made exitSimLoop() more flexible to enable some of these.


Diffs
-

  src/sim/pseudo_inst.cc 7338bc628489 
  src/sim/sim_events.cc 7338bc628489 
  src/sim/sim_exit.hh 7338bc628489 

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


Testing
---


Thanks,

Steve

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

[m5-dev] Review Request: sim: delete unused CheckSwapEvent code.


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

Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan 
Binkert.


Summary
---

sim: delete unused CheckSwapEvent code.
There's no way to even create one of these anymore.


Diffs
-

  src/sim/sim_events.hh 7338bc628489 
  src/sim/sim_events.cc 7338bc628489 

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


Testing
---


Thanks,

Steve

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

[m5-dev] Review Request: sim: clean up CountedDrainEvent slightly.


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

Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan 
Binkert.


Summary
---

sim: clean up CountedDrainEvent slightly.
There's no reason for it to derive from SimLoopExitEvent.
This whole drain thing needs to be redone eventually,
but this is a stopgap to make later changes to
SimLoopExitEvent feasible.


Diffs
-

  src/sim/sim_events.hh 7338bc628489 
  src/sim/sim_events.cc 7338bc628489 

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


Testing
---


Thanks,

Steve

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

[m5-dev] Review Request: Hammer protocol: Change how data to copied from L1 to L2 cache and vice-versa


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

Review request for Default.


Summary
---

This patch changes the manner in which data is copied from L1 to L2 cache
in the implementation of the Hammer's cache coherence protocol. Earlier, 
data was copied directly from one cache entry to another. This has been
broken in to two parts. First, the data is copied from the source cache
entry to a transaction buffer entry. Then, data is copied from the
transaction buffer entry to the destination cache entry.

This has been done to maintain the invariant - at any given instant, a
multiple caches under a controller are exclusive with respect to each 
other.


Diffs
-

  src/mem/protocol/MOESI_hammer-cache.sm UNKNOWN 

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


Testing
---

The changes have been tested with ruby random tester using a single seed for 
100,000 loads and number of processors = 1, 2, 4 and 8.


Thanks,

Nilay

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

Re: [m5-dev] Review Request: inorder: replace schedEvent() code with reschedule().


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

Ship it!


Looks good.  I assume it works.

- Nathan


On 2011-01-04 14:41:12, Steve Reinhardt wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.m5sim.org/r/369/
 ---
 
 (Updated 2011-01-04 14:41:12)
 
 
 Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
 Nathan Binkert.
 
 
 Summary
 ---
 
 inorder: replace schedEvent() code with reschedule().
 There were several copies of similar functions that looked
 like they all replicated reschedule(), so I replaced them
 with direct calls.  Keeping this separate from the previous
 cset since there may be some subtle functional differences
 if the code ever reschedules an event that is scheduled but
 not squashed (though none were detected in the regressions).
 
 
 Diffs
 -
 
   src/cpu/inorder/cpu.hh 7338bc628489 
   src/cpu/inorder/cpu.cc 7338bc628489 
   src/cpu/inorder/resource.cc 7338bc628489 
   src/cpu/inorder/resource_pool.cc 7338bc628489 
 
 Diff: http://reviews.m5sim.org/r/369/diff
 
 
 Testing
 ---
 
 
 Thanks,
 
 Steve
 


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

[m5-dev] Review Request: stats: rename StatEvent() function to schedStatEvent().


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

Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan 
Binkert.


Summary
---

stats: rename StatEvent() function to schedStatEvent().
This follows the style rules and is more descriptive.


Diffs
-

  src/python/m5/stats.py 7338bc628489 
  src/python/swig/stats.i 7338bc628489 
  src/sim/pseudo_inst.cc 7338bc628489 
  src/sim/simulate.cc 7338bc628489 
  src/sim/stat_control.hh 7338bc628489 
  src/sim/stat_control.cc 7338bc628489 

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


Testing
---


Thanks,

Steve

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

[m5-dev] Review Request: Replace curTick global variable with accessor functions.


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

Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan 
Binkert.


Summary
---

Replace curTick global variable with accessor functions.
This step makes it easy to replace the accessor functions
(which still access a global variable) with ones that access
per-thread curTick values.


Diffs
-

  src/arch/alpha/isa/decoder.isa 7338bc628489 
  src/arch/alpha/kernel_stats.cc 7338bc628489 
  src/arch/alpha/tru64/process.cc 7338bc628489 
  src/arch/arm/table_walker.cc 7338bc628489 
  src/arch/mips/isa.cc 7338bc628489 
  src/arch/mips/isa/formats/mt.isa 7338bc628489 
  src/arch/mips/locked_mem.hh 7338bc628489 
  src/arch/mips/mt.hh 7338bc628489 
  src/arch/sparc/ua2005.cc 7338bc628489 
  src/arch/x86/interrupts.cc 7338bc628489 
  src/base/cp_annotate.hh 7338bc628489 
  src/base/cp_annotate.cc 7338bc628489 
  src/base/fast_alloc.cc 7338bc628489 
  src/base/misc.cc 7338bc628489 
  src/base/remote_gdb.cc 7338bc628489 
  src/base/statistics.hh 7338bc628489 
  src/base/stats/mysql.cc 7338bc628489 
  src/base/stats/output.cc 7338bc628489 
  src/base/trace.hh 7338bc628489 
  src/cpu/base.hh 7338bc628489 
  src/cpu/base.cc 7338bc628489 
  src/cpu/checker/cpu.cc 7338bc628489 
  src/cpu/checker/cpu_impl.hh 7338bc628489 
  src/cpu/inorder/cpu.hh 7338bc628489 
  src/cpu/inorder/cpu.cc 7338bc628489 
  src/cpu/inorder/inorder_dyn_inst.cc 7338bc628489 
  src/cpu/inorder/pipeline_stage.cc 7338bc628489 
  src/cpu/inorder/reg_dep_map.cc 7338bc628489 
  src/cpu/inorder/resource.cc 7338bc628489 
  src/cpu/inorder/resource_pool.9stage.cc 7338bc628489 
  src/cpu/inorder/resource_pool.cc 7338bc628489 
  src/cpu/inorder/resources/branch_predictor.cc 7338bc628489 
  src/cpu/inorder/resources/cache_unit.cc 7338bc628489 
  src/cpu/inorder/resources/execution_unit.cc 7338bc628489 
  src/cpu/inorder/resources/fetch_seq_unit.cc 7338bc628489 
  src/cpu/inorder/resources/graduation_unit.cc 7338bc628489 
  src/cpu/inorder/resources/mult_div_unit.cc 7338bc628489 
  src/cpu/o3/commit_impl.hh 7338bc628489 
  src/cpu/o3/cpu.hh 7338bc628489 
  src/cpu/o3/cpu.cc 7338bc628489 
  src/cpu/o3/fetch_impl.hh 7338bc628489 
  src/cpu/o3/inst_queue_impl.hh 7338bc628489 
  src/cpu/o3/lsq_impl.hh 7338bc628489 
  src/cpu/o3/lsq_unit.hh 7338bc628489 
  src/cpu/o3/lsq_unit_impl.hh 7338bc628489 
  src/cpu/o3/thread_context_impl.hh 7338bc628489 
  src/cpu/ozone/back_end.hh 7338bc628489 
  src/cpu/ozone/cpu.hh 7338bc628489 
  src/cpu/ozone/cpu_impl.hh 7338bc628489 
  src/cpu/ozone/front_end_impl.hh 7338bc628489 
  src/cpu/ozone/inorder_back_end.hh 7338bc628489 
  src/cpu/ozone/inst_queue_impl.hh 7338bc628489 
  src/cpu/ozone/lsq_unit.hh 7338bc628489 
  src/cpu/ozone/lsq_unit_impl.hh 7338bc628489 
  src/cpu/ozone/lw_back_end_impl.hh 7338bc628489 
  src/cpu/ozone/lw_lsq.hh 7338bc628489 
  src/cpu/ozone/lw_lsq_impl.hh 7338bc628489 
  src/cpu/pc_event.cc 7338bc628489 
  src/cpu/simple/atomic.cc 7338bc628489 
  src/cpu/simple/base.cc 7338bc628489 
  src/cpu/simple/timing.cc 7338bc628489 
  src/cpu/simple_thread.cc 7338bc628489 
  src/cpu/static_inst.cc 7338bc628489 
  src/cpu/testers/directedtest/RubyDirectedTester.cc 7338bc628489 
  src/cpu/testers/memtest/memtest.cc 7338bc628489 
  src/cpu/testers/rubytest/Check.cc 7338bc628489 
  src/cpu/testers/rubytest/RubyTester.cc 7338bc628489 
  src/cpu/trace/trace_cpu.cc 7338bc628489 
  src/dev/alpha/backdoor.cc 7338bc628489 
  src/dev/arm/pl011.cc 7338bc628489 
  src/dev/arm/pl111.cc 7338bc628489 
  src/dev/arm/rv_ctrl.cc 7338bc628489 
  src/dev/arm/timer_sp804.cc 7338bc628489 
  src/dev/etherbus.cc 7338bc628489 
  src/dev/etherdump.cc 7338bc628489 
  src/dev/etherlink.cc 7338bc628489 
  src/dev/ethertap.cc 7338bc628489 
  src/dev/i8254xGBe.cc 7338bc628489 
  src/dev/ide_disk.cc 7338bc628489 
  src/dev/intel_8254_timer.cc 7338bc628489 
  src/dev/io_device.cc 7338bc628489 
  src/dev/mc146818.hh 7338bc628489 
  src/dev/mc146818.cc 7338bc628489 
  src/dev/ns_gige.cc 7338bc628489 
  src/dev/sinic.cc 7338bc628489 
  src/dev/uart8250.cc 7338bc628489 
  src/kern/kernel_stats.cc 7338bc628489 
  src/mem/bridge.cc 7338bc628489 
  src/mem/bus.cc 7338bc628489 
  src/mem/cache/base.hh 7338bc628489 
  src/mem/cache/base.cc 7338bc628489 
  src/mem/cache/blk.hh 7338bc628489 
  src/mem/cache/cache_impl.hh 7338bc628489 
  src/mem/cache/mshr.hh 7338bc628489 
  src/mem/cache/mshr.cc 7338bc628489 
  src/mem/cache/mshr_queue.hh 7338bc628489 
  src/mem/cache/tags/fa_lru.cc 7338bc628489 
  src/mem/cache/tags/iic.cc 7338bc628489 
  src/mem/cache/tags/lru.cc 7338bc628489 
  src/mem/dram.cc 7338bc628489 
  src/mem/mport.cc 7338bc628489 
  src/mem/packet.hh 7338bc628489 
  src/mem/request.hh 7338bc628489 
  src/mem/ruby/eventqueue/RubyEventQueue.hh 7338bc628489 
  src/mem/ruby/system/RubyPort.cc 7338bc628489

Re: [m5-dev] Review Request: pseudoinst: get rid of mainEventQueue references.


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

Ship it!


Looks good.

- Nathan


On 2011-01-04 14:41:43, Steve Reinhardt wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.m5sim.org/r/370/
 ---
 
 (Updated 2011-01-04 14:41:43)
 
 
 Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
 Nathan Binkert.
 
 
 Summary
 ---
 
 pseudoinst: get rid of mainEventQueue references.
 Avoid direct references to mainEventQueue in pseudo-insts
 by indirecting through associated CPU object.
 Made exitSimLoop() more flexible to enable some of these.
 
 
 Diffs
 -
 
   src/sim/pseudo_inst.cc 7338bc628489 
   src/sim/sim_events.cc 7338bc628489 
   src/sim/sim_exit.hh 7338bc628489 
 
 Diff: http://reviews.m5sim.org/r/370/diff
 
 
 Testing
 ---
 
 
 Thanks,
 
 Steve
 


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

Re: [m5-dev] Review Request: sim: delete unused CheckSwapEvent code.


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

Ship it!


- Nathan


On 2011-01-04 14:41:53, Steve Reinhardt wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.m5sim.org/r/371/
 ---
 
 (Updated 2011-01-04 14:41:53)
 
 
 Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
 Nathan Binkert.
 
 
 Summary
 ---
 
 sim: delete unused CheckSwapEvent code.
 There's no way to even create one of these anymore.
 
 
 Diffs
 -
 
   src/sim/sim_events.hh 7338bc628489 
   src/sim/sim_events.cc 7338bc628489 
 
 Diff: http://reviews.m5sim.org/r/371/diff
 
 
 Testing
 ---
 
 
 Thanks,
 
 Steve
 


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

[m5-dev] recent batch of patches to review

FYI, I've been working recently on implementing multiple event
queues/threads, and while the actual multi-queue code isn't ready yet, these
patches represent a bunch of preliminary cleanup/setup changes that make it
possible.  I'd like to get these committed so I don't have to keep rebasing
them... especially the curTick accessor one.

Thanks,

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

Re: [m5-dev] Review Request: inorder: get rid of references to mainEventQueue.


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

Ship it!


- Nathan


On 2011-01-04 14:40:57, Steve Reinhardt wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.m5sim.org/r/368/
 ---
 
 (Updated 2011-01-04 14:40:57)
 
 
 Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
 Nathan Binkert.
 
 
 Summary
 ---
 
 inorder: get rid of references to mainEventQueue.
 Events need to be scheduled on the queue assigned
 to the SimObject, not on the global queue (which
 should be going away).
 Also cleaned up a number of redundant expressions
 that made the code unnecessarily verbose.
 
 
 Diffs
 -
 
   src/cpu/inorder/cpu.hh 7338bc628489 
   src/cpu/inorder/cpu.cc 7338bc628489 
   src/cpu/inorder/resource.hh 7338bc628489 
   src/cpu/inorder/resource.cc 7338bc628489 
   src/cpu/inorder/resource_pool.cc 7338bc628489 
 
 Diff: http://reviews.m5sim.org/r/368/diff
 
 
 Testing
 ---
 
 
 Thanks,
 
 Steve
 


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

Re: [m5-dev] recent batch of patches to review

2011-01-04 Thread nathan binkert

 FYI, I've been working recently on implementing multiple event
 queues/threads, and while the actual multi-queue code isn't ready yet, these
 patches represent a bunch of preliminary cleanup/setup changes that make it
 possible.  I'd like to get these committed so I don't have to keep rebasing
 them... especially the curTick accessor one.

A general request for everyone.  If you have a review where most of
the code is done by a script and a little bit matters, can you
separate the automated part into a separate diff so it's easier to
find the important stuff?  e.g. for the sort includes diff, I had
three.  The util, the util being run, and the fixup after the util was
run.  (This makes rebasing easier too because you can just redo the
big diff).  They can be folded for the final (though probably better
if not.)

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

Re: [m5-dev] Review Request: Replace curTick global variable with accessor functions.


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

Ship it!


Maybe it's too much for this diff, but I think you can get rid of the global 
variable and put _curTick in the event queue class.  Then you can put a 
curTick() accessor into the EventQueue and EventManager classes, and create a 
global curTick (though maybe with a different name like mainCurTick() so we can 
tell what is what) that accesses mainEventQueue.curTick().  Maybe not needed 
for this diff, but certainly something that would seem to be a good step.  
Thanks for doing this.  I've been wanting to do exactly this for a while.

- Nathan


On 2011-01-04 14:43:38, Steve Reinhardt wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.m5sim.org/r/374/
 ---
 
 (Updated 2011-01-04 14:43:38)
 
 
 Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
 Nathan Binkert.
 
 
 Summary
 ---
 
 Replace curTick global variable with accessor functions.
 This step makes it easy to replace the accessor functions
 (which still access a global variable) with ones that access
 per-thread curTick values.
 
 
 Diffs
 -
 
   src/arch/alpha/isa/decoder.isa 7338bc628489 
   src/arch/alpha/kernel_stats.cc 7338bc628489 
   src/arch/alpha/tru64/process.cc 7338bc628489 
   src/arch/arm/table_walker.cc 7338bc628489 
   src/arch/mips/isa.cc 7338bc628489 
   src/arch/mips/isa/formats/mt.isa 7338bc628489 
   src/arch/mips/locked_mem.hh 7338bc628489 
   src/arch/mips/mt.hh 7338bc628489 
   src/arch/sparc/ua2005.cc 7338bc628489 
   src/arch/x86/interrupts.cc 7338bc628489 
   src/base/cp_annotate.hh 7338bc628489 
   src/base/cp_annotate.cc 7338bc628489 
   src/base/fast_alloc.cc 7338bc628489 
   src/base/misc.cc 7338bc628489 
   src/base/remote_gdb.cc 7338bc628489 
   src/base/statistics.hh 7338bc628489 
   src/base/stats/mysql.cc 7338bc628489 
   src/base/stats/output.cc 7338bc628489 
   src/base/trace.hh 7338bc628489 
   src/cpu/base.hh 7338bc628489 
   src/cpu/base.cc 7338bc628489 
   src/cpu/checker/cpu.cc 7338bc628489 
   src/cpu/checker/cpu_impl.hh 7338bc628489 
   src/cpu/inorder/cpu.hh 7338bc628489 
   src/cpu/inorder/cpu.cc 7338bc628489 
   src/cpu/inorder/inorder_dyn_inst.cc 7338bc628489 
   src/cpu/inorder/pipeline_stage.cc 7338bc628489 
   src/cpu/inorder/reg_dep_map.cc 7338bc628489 
   src/cpu/inorder/resource.cc 7338bc628489 
   src/cpu/inorder/resource_pool.9stage.cc 7338bc628489 
   src/cpu/inorder/resource_pool.cc 7338bc628489 
   src/cpu/inorder/resources/branch_predictor.cc 7338bc628489 
   src/cpu/inorder/resources/cache_unit.cc 7338bc628489 
   src/cpu/inorder/resources/execution_unit.cc 7338bc628489 
   src/cpu/inorder/resources/fetch_seq_unit.cc 7338bc628489 
   src/cpu/inorder/resources/graduation_unit.cc 7338bc628489 
   src/cpu/inorder/resources/mult_div_unit.cc 7338bc628489 
   src/cpu/o3/commit_impl.hh 7338bc628489 
   src/cpu/o3/cpu.hh 7338bc628489 
   src/cpu/o3/cpu.cc 7338bc628489 
   src/cpu/o3/fetch_impl.hh 7338bc628489 
   src/cpu/o3/inst_queue_impl.hh 7338bc628489 
   src/cpu/o3/lsq_impl.hh 7338bc628489 
   src/cpu/o3/lsq_unit.hh 7338bc628489 
   src/cpu/o3/lsq_unit_impl.hh 7338bc628489 
   src/cpu/o3/thread_context_impl.hh 7338bc628489 
   src/cpu/ozone/back_end.hh 7338bc628489 
   src/cpu/ozone/cpu.hh 7338bc628489 
   src/cpu/ozone/cpu_impl.hh 7338bc628489 
   src/cpu/ozone/front_end_impl.hh 7338bc628489 
   src/cpu/ozone/inorder_back_end.hh 7338bc628489 
   src/cpu/ozone/inst_queue_impl.hh 7338bc628489 
   src/cpu/ozone/lsq_unit.hh 7338bc628489 
   src/cpu/ozone/lsq_unit_impl.hh 7338bc628489 
   src/cpu/ozone/lw_back_end_impl.hh 7338bc628489 
   src/cpu/ozone/lw_lsq.hh 7338bc628489 
   src/cpu/ozone/lw_lsq_impl.hh 7338bc628489 
   src/cpu/pc_event.cc 7338bc628489 
   src/cpu/simple/atomic.cc 7338bc628489 
   src/cpu/simple/base.cc 7338bc628489 
   src/cpu/simple/timing.cc 7338bc628489 
   src/cpu/simple_thread.cc 7338bc628489 
   src/cpu/static_inst.cc 7338bc628489 
   src/cpu/testers/directedtest/RubyDirectedTester.cc 7338bc628489 
   src/cpu/testers/memtest/memtest.cc 7338bc628489 
   src/cpu/testers/rubytest/Check.cc 7338bc628489 
   src/cpu/testers/rubytest/RubyTester.cc 7338bc628489 
   src/cpu/trace/trace_cpu.cc 7338bc628489 
   src/dev/alpha/backdoor.cc 7338bc628489 
   src/dev/arm/pl011.cc 7338bc628489 
   src/dev/arm/pl111.cc 7338bc628489 
   src/dev/arm/rv_ctrl.cc 7338bc628489 
   src/dev/arm/timer_sp804.cc 7338bc628489 
   src/dev/etherbus.cc 7338bc628489 
   src/dev/etherdump.cc 7338bc628489 
   src/dev/etherlink.cc 7338bc628489 
   src/dev/ethertap.cc 7338bc628489 
   src/dev/i8254xGBe.cc 7338bc628489 
   src/dev/ide_disk.cc 7338bc628489 
   src/dev/intel_8254_timer.cc 7338bc628489

Re: [m5-dev] Review Request: ruby: get rid of ruby's Debug.hh


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

(Updated 2011-01-04 15:02:38.269001)


Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan 
Binkert.


Summary
---

ruby: get rid of ruby's Debug.hh

Get rid of the Debug class
Get rid of ASSERT and use assert
Use DPRINTF for ProtocolTrace


Diffs (updated)
-

  configs/ruby/Ruby.py 7338bc628489 
  src/mem/SConscript 7338bc628489 
  src/mem/ruby/buffers/MessageBuffer.hh 7338bc628489 
  src/mem/ruby/buffers/MessageBuffer.cc 7338bc628489 
  src/mem/ruby/common/Debug.hh 7338bc628489 
  src/mem/ruby/common/Debug.cc 7338bc628489 
  src/mem/ruby/common/Debug.py 7338bc628489 
  src/mem/ruby/common/Global.hh 7338bc628489 
  src/mem/ruby/common/Global.cc 7338bc628489 
  src/mem/ruby/common/SConscript 7338bc628489 
  src/mem/ruby/common/Set.cc 7338bc628489 
  src/mem/ruby/eventqueue/RubyEventQueue.cc 7338bc628489 
  src/mem/ruby/filters/BulkBloomFilter.cc 7338bc628489 
  src/mem/ruby/network/garnet/fixed-pipeline/GarnetNetwork_d.cc 7338bc628489 
  src/mem/ruby/network/garnet/fixed-pipeline/NetworkInterface_d.cc 7338bc628489 
  src/mem/ruby/network/garnet/fixed-pipeline/flit_d.hh 7338bc628489 
  src/mem/ruby/network/garnet/flexible-pipeline/GarnetNetwork.cc 7338bc628489 
  src/mem/ruby/network/garnet/flexible-pipeline/NetworkInterface.cc 
7338bc628489 
  src/mem/ruby/network/garnet/flexible-pipeline/flit.hh 7338bc628489 
  src/mem/ruby/network/simple/SimpleNetwork.cc 7338bc628489 
  src/mem/ruby/network/simple/Throttle.cc 7338bc628489 
  src/mem/ruby/network/simple/Topology.cc 7338bc628489 
  src/mem/ruby/profiler/Profiler.hh 7338bc628489 
  src/mem/ruby/profiler/Profiler.cc 7338bc628489 
  src/mem/ruby/slicc_interface/RubySlicc_Util.hh 7338bc628489 
  src/mem/ruby/storebuffer/storebuffer.cc 7338bc628489 
  src/mem/ruby/system/RubySystem.py 7338bc628489 
  src/mem/ruby/system/Sequencer.cc 7338bc628489 
  src/mem/ruby/system/System.cc 7338bc628489 
  src/mem/ruby/tester/DeterministicDriver.cc 7338bc628489 
  src/mem/ruby/tester/RaceyDriver.cc 7338bc628489 
  src/mem/ruby/tester/RaceyPseudoThread.cc 7338bc628489 
  src/mem/ruby/tester/test_framework.cc 7338bc628489 
  src/mem/slicc/symbols/StateMachine.py 7338bc628489 
  src/mem/slicc/symbols/Type.py 7338bc628489 

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


Testing
---

This compiles and passes all of the quick regressions, but it would be nice for 
a Ruby developer to take a look and see if I got rid of any useful 
functionality.


Thanks,

Nathan

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

Re: [m5-dev] recent batch of patches to review