Discussion:
Review Request: Updating MOESI CMP Directory protocol as per the new interface
(too old to reply)
Nilay Vaish
2010-12-23 05:00:59 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/359/
-----------------------------------------------------------

Review request for Default.


Summary
-------

This is a request for reviewing the proposed changes to the MOESI CMP directory cache coherence protocol to make it conform with the new cache memory interface and changes to SLICC.


Diffs
-----

src/mem/protocol/MOESI_CMP_directory-dir.sm UNKNOWN
src/mem/protocol/MOESI_CMP_directory-dma.sm UNKNOWN
src/mem/protocol/MOESI_CMP_directory-L2cache.sm UNKNOWN
src/mem/protocol/MOESI_CMP_directory-L1cache.sm UNKNOWN

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


Testing
-------

These changes have been tested using the Ruby random tester. The tester was used with -l = 1048576 and -n = 2.


Thanks,

Nilay
Nilay Vaish
2010-12-24 17:02:20 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/359/
-----------------------------------------------------------

(Updated 2010-12-24 09:02:20.653744)


Review request for Default.


Changes
-------

is_valid_cache_entry() and is_valid_tbe() have been removed. These have been replaced with a call to is_valid_ptr(<ptr variable>).


Summary
-------

This is a request for reviewing the proposed changes to the MOESI CMP directory cache coherence protocol to make it conform with the new cache memory interface and changes to SLICC.


Diffs (updated)
-----

src/mem/protocol/MOESI_CMP_directory-L1cache.sm UNKNOWN
src/mem/protocol/MOESI_CMP_directory-L2cache.sm UNKNOWN
src/mem/protocol/MOESI_CMP_directory-dir.sm UNKNOWN
src/mem/protocol/MOESI_CMP_directory-dma.sm UNKNOWN

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


Testing
-------

These changes have been tested using the Ruby random tester. The tester was used with -l = 1048576 and -n = 2.


Thanks,

Nilay
Nilay Vaish
2011-01-01 01:35:16 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/359/
-----------------------------------------------------------

(Updated 2010-12-31 17:35:16.274524)


Review request for Default.


Changes
-------

The changes made were necessitated by the changes made to SLICC and CacheMemory class.


Summary
-------

This is a request for reviewing the proposed changes to the MOESI CMP directory cache coherence protocol to make it conform with the new cache memory interface and changes to SLICC.


Diffs (updated)
-----

src/mem/protocol/MOESI_CMP_directory-L1cache.sm UNKNOWN
src/mem/protocol/MOESI_CMP_directory-L2cache.sm UNKNOWN
src/mem/protocol/MOESI_CMP_directory-dir.sm UNKNOWN
src/mem/protocol/MOESI_CMP_directory-dma.sm UNKNOWN

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


Testing
-------

These changes have been tested using the Ruby random tester. The tester was used with -l = 1048576 and -n = 2.


Thanks,

Nilay
Steve Reinhardt
2011-01-03 18:16:16 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/359/#review586
-----------------------------------------------------------



src/mem/protocol/MOESI_CMP_directory-L2cache.sm
<http://reviews.m5sim.org/r/359/#comment824>

As usual, please delete unused code instead of commenting it out.


- Steve
Post by Nilay Vaish
-----------------------------------------------------------
http://reviews.m5sim.org/r/359/
-----------------------------------------------------------
(Updated 2010-12-31 17:35:16)
Review request for Default.
Summary
-------
This is a request for reviewing the proposed changes to the MOESI CMP directory cache coherence protocol to make it conform with the new cache memory interface and changes to SLICC.
Diffs
-----
src/mem/protocol/MOESI_CMP_directory-L1cache.sm UNKNOWN
src/mem/protocol/MOESI_CMP_directory-L2cache.sm UNKNOWN
src/mem/protocol/MOESI_CMP_directory-dir.sm UNKNOWN
src/mem/protocol/MOESI_CMP_directory-dma.sm UNKNOWN
Diff: http://reviews.m5sim.org/r/359/diff
Testing
-------
These changes have been tested using the Ruby random tester. The tester was used with -l = 1048576 and -n = 2.
Thanks,
Nilay
Nilay Vaish
2011-01-03 22:20:22 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/359/
-----------------------------------------------------------

(Updated 2011-01-03 14:20:22.706038)


Review request for Default.


Changes
-------

Removed the code that was commented out in the previous revision.


Summary
-------

This is a request for reviewing the proposed changes to the MOESI CMP directory cache coherence protocol to make it conform with the new cache memory interface and changes to SLICC.


Diffs (updated)
-----

src/mem/protocol/MOESI_CMP_directory-L1cache.sm UNKNOWN
src/mem/protocol/MOESI_CMP_directory-L2cache.sm UNKNOWN
src/mem/protocol/MOESI_CMP_directory-dir.sm UNKNOWN
src/mem/protocol/MOESI_CMP_directory-dma.sm UNKNOWN

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


Testing
-------

These changes have been tested using the Ruby random tester. The tester was used with -l = 1048576 and -n = 2.


Thanks,

Nilay
Nilay Vaish
2011-01-06 14:52:27 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/359/
-----------------------------------------------------------

(Updated 2011-01-06 06:52:27.512216)


Review request for Default.


Summary
-------

This is a request for reviewing the proposed changes to the MOESI CMP directory cache coherence protocol to make it conform with the new cache memory interface and changes to SLICC.


Diffs (updated)
-----

src/mem/protocol/MOESI_CMP_directory-dir.sm UNKNOWN
src/mem/protocol/MOESI_CMP_directory-dma.sm UNKNOWN
src/mem/protocol/MOESI_CMP_directory-L2cache.sm UNKNOWN
src/mem/protocol/MOESI_CMP_directory-L1cache.sm UNKNOWN

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


Testing
-------

These changes have been tested using the Ruby random tester. The tester was used with -l = 1048576 and -n = 2.


Thanks,

Nilay
Nilay Vaish
2011-01-13 06:43:56 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/359/
-----------------------------------------------------------

(Updated 2011-01-12 22:43:56.382578)


Review request for Default.


Summary
-------

This is a request for reviewing the proposed changes to the MOESI CMP directory cache coherence protocol to make it conform with the new cache memory interface and changes to SLICC.


Diffs (updated)
-----

src/mem/protocol/MOESI_CMP_directory-L1cache.sm c6bc8fe81e79
src/mem/protocol/MOESI_CMP_directory-L2cache.sm c6bc8fe81e79
src/mem/protocol/MOESI_CMP_directory-dir.sm c6bc8fe81e79
src/mem/protocol/MOESI_CMP_directory-dma.sm c6bc8fe81e79

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


Testing
-------

These changes have been tested using the Ruby random tester. The tester was used with -l = 1048576 and -n = 2.


Thanks,

Nilay
Nilay Vaish
2011-01-13 06:44:50 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/359/
-----------------------------------------------------------

(Updated 2011-01-12 22:44:50.684699)


Review request for Default.


Summary
-------

This is a request for reviewing the proposed changes to the MOESI CMP directory cache coherence protocol to make it conform with the new cache memory interface and changes to SLICC.


Diffs (updated)
-----

src/mem/protocol/MOESI_CMP_directory-L1cache.sm c6bc8fe81e79
src/mem/protocol/MOESI_CMP_directory-L2cache.sm c6bc8fe81e79
src/mem/protocol/MOESI_CMP_directory-dir.sm c6bc8fe81e79
src/mem/protocol/MOESI_CMP_directory-dma.sm c6bc8fe81e79

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


Testing
-------

These changes have been tested using the Ruby random tester. The tester was used with -l = 1048576 and -n = 2.


Thanks,

Nilay
Brad Beckmann
2011-01-13 16:48:29 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/359/#review728
-----------------------------------------------------------

Ship it!


Well Done! I have a couple questions just to clarify my understanding of what is going on, but overall this looks great to me.


src/mem/protocol/MOESI_CMP_directory-L1cache.sm
<http://reviews.m5sim.org/r/359/#comment992>

So the assumption here is the L1IcacheMemory.lookup() call either returns the L1I cache entry or NULL/OOD, correct? Does SLICC also support explicitly passing back OOD?



src/mem/protocol/MOESI_CMP_directory-L1cache.sm
<http://reviews.m5sim.org/r/359/#comment993>

This seems like an unrelated change, correct. However it is pretty minor, so don't worry about it.


- Brad
Post by Nilay Vaish
-----------------------------------------------------------
http://reviews.m5sim.org/r/359/
-----------------------------------------------------------
(Updated 2011-01-12 22:44:50)
Review request for Default.
Summary
-------
This is a request for reviewing the proposed changes to the MOESI CMP directory cache coherence protocol to make it conform with the new cache memory interface and changes to SLICC.
Diffs
-----
src/mem/protocol/MOESI_CMP_directory-L1cache.sm c6bc8fe81e79
src/mem/protocol/MOESI_CMP_directory-L2cache.sm c6bc8fe81e79
src/mem/protocol/MOESI_CMP_directory-dir.sm c6bc8fe81e79
src/mem/protocol/MOESI_CMP_directory-dma.sm c6bc8fe81e79
Diff: http://reviews.m5sim.org/r/359/diff
Testing
-------
These changes have been tested using the Ruby random tester. The tester was used with -l = 1048576 and -n = 2.
Thanks,
Nilay
Nilay Vaish
2011-01-13 16:57:06 UTC
Permalink
Post by Nilay Vaish
src/mem/protocol/MOESI_CMP_directory-L1cache.sm, line 159
<http://reviews.m5sim.org/r/359/diff/8/?file=9537#file9537line159>
So the assumption here is the L1IcacheMemory.lookup() call either returns the L1I cache entry or NULL/OOD, correct? Does SLICC also support explicitly passing back OOD?
Currently, SLICC does not have support for Out Of Domain (OOD) token. But I can add that as I had done earlier. I am not sure if we actually need it.
Post by Nilay Vaish
src/mem/protocol/MOESI_CMP_directory-L1cache.sm, line 465
<http://reviews.m5sim.org/r/359/diff/8/?file=9537#file9537line465>
This seems like an unrelated change, correct. However it is pretty minor, so don't worry about it.
IIRC, this is necessary or else a certain panic state is reached. I think I should separately make this change.


- Nilay


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/359/#review728
-----------------------------------------------------------
Post by Nilay Vaish
-----------------------------------------------------------
http://reviews.m5sim.org/r/359/
-----------------------------------------------------------
(Updated 2011-01-12 22:44:50)
Review request for Default.
Summary
-------
This is a request for reviewing the proposed changes to the MOESI CMP directory cache coherence protocol to make it conform with the new cache memory interface and changes to SLICC.
Diffs
-----
src/mem/protocol/MOESI_CMP_directory-L1cache.sm c6bc8fe81e79
src/mem/protocol/MOESI_CMP_directory-L2cache.sm c6bc8fe81e79
src/mem/protocol/MOESI_CMP_directory-dir.sm c6bc8fe81e79
src/mem/protocol/MOESI_CMP_directory-dma.sm c6bc8fe81e79
Diff: http://reviews.m5sim.org/r/359/diff
Testing
-------
These changes have been tested using the Ruby random tester. The tester was used with -l = 1048576 and -n = 2.
Thanks,
Nilay
Beckmann, Brad
2011-01-13 17:05:07 UTC
Permalink
Hi Nilay,

Yes, please add the OOD token. I believe that will come in handy when developing new protocols. Don’t worry about separating out that RequestorMachine change. It seems like just a few extra lines. Also I believe the MOESI_CMP_Directory protocol did work correctly before your change, right? If so, the RequestorMachine lines are related to the rest of the patch.

Brad


From: Nilay Vaish [mailto:***@cs.wisc.edu]
Sent: Thursday, January 13, 2011 8:57 AM
To: Nilay Vaish; Default; Beckmann, Brad
Subject: Re: Review Request: Updating MOESI CMP Directory protocol as per the new interface

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



On January 13th, 2011, 8:48 a.m., Brad Beckmann wrote:
src/mem/protocol/MOESI_CMP_directory-L1cache.sm<http://reviews.m5sim.org/r/359/diff/8/?file=9537#file9537line159> (Diff revision 8)


155


if (L1DcacheMemory.isTagPresent(addr)) {

157


return L1Icache_entry;


So the assumption here is the L1IcacheMemory.lookup() call either returns the L1I cache entry or NULL/OOD, correct? Does SLICC also support explicitly passing back OOD?

Currently, SLICC does not have support for Out Of Domain (OOD) token. But I can add that as I had done earlier. I am not sure if we actually need it.


On January 13th, 2011, 8:48 a.m., Brad Beckmann wrote:
src/mem/protocol/MOESI_CMP_directory-L1cache.sm<http://reviews.m5sim.org/r/359/diff/8/?file=9537#file9537line465> (Diff revision 8)



430


out_msg.RequestorMachine := MachineType:L1Cache;


This seems like an unrelated change, correct. However it is pretty minor, so don't worry about it.

IIRC, this is necessary or else a certain panic state is reached. I think I should separately make this change.


- Nilay


On January 12th, 2011, 10:44 p.m., Nilay Vaish wrote:
Review request for Default.
By Nilay Vaish.

Updated 2011-01-12 22:44:50

Description

This is a request for reviewing the proposed changes to the MOESI CMP directory cache coherence protocol to make it conform with the new cache memory interface and changes to SLICC.


Testing

These changes have been tested using the Ruby random tester. The tester was used with -l = 1048576 and -n = 2.


Diffs

* src/mem/protocol/MOESI_CMP_directory-L1cache.sm (c6bc8fe81e79)
* src/mem/protocol/MOESI_CMP_directory-L2cache.sm (c6bc8fe81e79)
* src/mem/protocol/MOESI_CMP_directory-dir.sm (c6bc8fe81e79)
* src/mem/protocol/MOESI_CMP_directory-dma.sm (c6bc8fe81e79)

View Diff<http://reviews.m5sim.org/r/359/diff/>
Nilay Vaish
2011-01-13 17:17:19 UTC
Permalink
Brad, I will make all the changes, test the protocols and then post the
patches again. I will also update the other protocols. I don't think the
patch for MOESI_CMP_directory would need an update, but SLICC patch would
surely need to be updated. I get this done soon.

Thanks
Nilay
Post by Beckmann, Brad
Hi Nilay,
Yes, please add the OOD token. I believe that will come in handy when
developing new protocols. Don’t worry about separating out that
RequestorMachine change. It seems like just a few extra lines. Also I
believe the MOESI_CMP_Directory protocol did work correctly before your
change, right? If so, the RequestorMachine lines are related to the
rest of the patch.
Brad
Sent: Thursday, January 13, 2011 8:57 AM
To: Nilay Vaish; Default; Beckmann, Brad
Subject: Re: Review Request: Updating MOESI CMP Directory protocol as per the new interface
This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/359/
src/mem/protocol/MOESI_CMP_directory-L1cache.sm<http://reviews.m5sim.org/r/359/diff/8/?file=9537#file9537line159> (Diff revision 8)
155
if (L1DcacheMemory.isTagPresent(addr)) {
157
return L1Icache_entry;
So the assumption here is the L1IcacheMemory.lookup() call either returns the L1I cache entry or NULL/OOD, correct? Does SLICC also support explicitly passing back OOD?
Currently, SLICC does not have support for Out Of Domain (OOD) token. But I can add that as I had done earlier. I am not sure if we actually need it.
src/mem/protocol/MOESI_CMP_directory-L1cache.sm<http://reviews.m5sim.org/r/359/diff/8/?file=9537#file9537line465> (Diff revision 8)
430
out_msg.RequestorMachine := MachineType:L1Cache;
This seems like an unrelated change, correct. However it is pretty minor, so don't worry about it.
IIRC, this is necessary or else a certain panic state is reached. I think I should separately make this change.
- Nilay
Review request for Default.
By Nilay Vaish.
Updated 2011-01-12 22:44:50
Description
This is a request for reviewing the proposed changes to the MOESI CMP directory cache coherence protocol to make it conform with the new cache memory interface and changes to SLICC.
Testing
These changes have been tested using the Ruby random tester. The tester was used with -l = 1048576 and -n = 2.
Diffs
* src/mem/protocol/MOESI_CMP_directory-L1cache.sm (c6bc8fe81e79)
* src/mem/protocol/MOESI_CMP_directory-L2cache.sm (c6bc8fe81e79)
* src/mem/protocol/MOESI_CMP_directory-dir.sm (c6bc8fe81e79)
* src/mem/protocol/MOESI_CMP_directory-dma.sm (c6bc8fe81e79)
View Diff<http://reviews.m5sim.org/r/359/diff/>
Nilay Vaish
2011-01-17 00:02:27 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/359/
-----------------------------------------------------------

(Updated 2011-01-16 16:02:27.482294)


Review request for Default.


Summary (updated)
-------

Ruby: Updates MOESI CMP directory protocol
This patch updates the MOESI CMP directory protocol to conform with the new
interfaces of CacheMemory and TBETable classes, and the changes in SLICC.


Diffs (updated)
-----

src/mem/protocol/MOESI_CMP_directory-L1cache.sm 696063d6ed04
src/mem/protocol/MOESI_CMP_directory-L2cache.sm 696063d6ed04
src/mem/protocol/MOESI_CMP_directory-dir.sm 696063d6ed04
src/mem/protocol/MOESI_CMP_directory-dma.sm 696063d6ed04

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


Testing
-------

These changes have been tested using the Ruby random tester. The tester was used with -l = 1048576 and -n = 2.


Thanks,

Nilay
Brad Beckmann
2011-01-17 15:51:49 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/359/#review752
-----------------------------------------------------------

Ship it!


Please merge it with the other patches.

- Brad
Post by Nilay Vaish
-----------------------------------------------------------
http://reviews.m5sim.org/r/359/
-----------------------------------------------------------
(Updated 2011-01-16 16:02:27)
Review request for Default.
Summary
-------
Ruby: Updates MOESI CMP directory protocol
This patch updates the MOESI CMP directory protocol to conform with the new
interfaces of CacheMemory and TBETable classes, and the changes in SLICC.
Diffs
-----
src/mem/protocol/MOESI_CMP_directory-L1cache.sm 696063d6ed04
src/mem/protocol/MOESI_CMP_directory-L2cache.sm 696063d6ed04
src/mem/protocol/MOESI_CMP_directory-dir.sm 696063d6ed04
src/mem/protocol/MOESI_CMP_directory-dma.sm 696063d6ed04
Diff: http://reviews.m5sim.org/r/359/diff
Testing
-------
These changes have been tested using the Ruby random tester. The tester was used with -l = 1048576 and -n = 2.
Thanks,
Nilay
Loading...