Discussion:
[gem5-dev] Review Request 2845: ruby: Expose MessageBuffers as SimObjects
(too old to reply)
Joel Hestness
2015-05-25 23:51:46 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2845/
-----------------------------------------------------------

Review request for Default.


Repository: gem5


Description
-------

Changeset 10850:0fc919c768b2
---------------------------
ruby: Expose MessageBuffers as SimObjects

Expose MessageBuffers from SLICC controllers as SimObjects that can be
manipulated in Python. This patch has numerous benefits:
1) First and foremost, it exposes MessageBuffers as SimObjects that can be
manipulated in Python code. This allows parameters to be set and checked in
Python code to avoid obfuscating parameters within protocol files. Further, now
as SimObjects, MessageBuffer parameters are printed to config output files as a
way to track parameters across simulations (e.g. buffer sizes)
2) Cleans up special-case code for responseFromMemory buffers, and aligns their
instantiation and use with mandatoryQueue buffers. These two special buffers
are the only MessageBuffers that are exposed to components outside of SLICC
controllers, and they're both slave ends of these buffers. They should be
exposed outside of SLICC in the same way, and this patch does it.
3) Removes the obfuscated port-like connection of MessageBuffers. Ports are
now connected in the init function of the generated controllers.


Diffs
-----

src/mem/protocol/RubySlicc_Defines.sm e61f847e74fd
src/mem/ruby/network/MessageBuffer.hh e61f847e74fd
src/mem/ruby/network/MessageBuffer.cc e61f847e74fd
src/mem/ruby/network/SConscript e61f847e74fd
src/mem/ruby/network/simple/SimpleNetwork.cc e61f847e74fd
src/mem/ruby/network/simple/Switch.cc e61f847e74fd
src/mem/ruby/slicc_interface/AbstractController.hh e61f847e74fd
src/mem/ruby/slicc_interface/AbstractController.cc e61f847e74fd
src/mem/slicc/symbols/StateMachine.py e61f847e74fd
src/python/swig/pyobject.cc e61f847e74fd

Diff: http://reviews.gem5.org/r/2845/diff/


Testing
-------

Regression tests with all Ruby protocols. Protocol file changes are in
separate patch (http://reviews.gem5.org/r/2844/) to keep this one slim.


Thanks,

Joel Hestness
Nilay Vaish
2015-05-26 17:11:28 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2845/#review6397
-----------------------------------------------------------


I went through both the patches. Unlikely that I am going to agree
these changes since we are dropping the functionality of connecting buffers
in python.


src/mem/ruby/network/simple/SimpleNetwork.cc (line 124)
<http://reviews.gem5.org/r/2845/#comment5496>

This is not right.



src/mem/ruby/network/simple/Switch.cc (line 99)
<http://reviews.gem5.org/r/2845/#comment5497>

This is not right either.



src/mem/ruby/slicc_interface/AbstractController.cc (line 62)
<http://reviews.gem5.org/r/2845/#comment5499>

Don't you need to set the receiver and the ordering property?



src/mem/ruby/slicc_interface/AbstractController.cc
<http://reviews.gem5.org/r/2845/#comment5498>

Do we not need to functionally write to the memory?


- Nilay Vaish
Post by Joel Hestness
-----------------------------------------------------------
http://reviews.gem5.org/r/2845/
-----------------------------------------------------------
(Updated May 25, 2015, 11:51 p.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 10850:0fc919c768b2
---------------------------
ruby: Expose MessageBuffers as SimObjects
Expose MessageBuffers from SLICC controllers as SimObjects that can be
1) First and foremost, it exposes MessageBuffers as SimObjects that can be
manipulated in Python code. This allows parameters to be set and checked in
Python code to avoid obfuscating parameters within protocol files. Further, now
as SimObjects, MessageBuffer parameters are printed to config output files as a
way to track parameters across simulations (e.g. buffer sizes)
2) Cleans up special-case code for responseFromMemory buffers, and aligns their
instantiation and use with mandatoryQueue buffers. These two special buffers
are the only MessageBuffers that are exposed to components outside of SLICC
controllers, and they're both slave ends of these buffers. They should be
exposed outside of SLICC in the same way, and this patch does it.
3) Removes the obfuscated port-like connection of MessageBuffers. Ports are
now connected in the init function of the generated controllers.
Diffs
-----
src/mem/protocol/RubySlicc_Defines.sm e61f847e74fd
src/mem/ruby/network/MessageBuffer.hh e61f847e74fd
src/mem/ruby/network/MessageBuffer.cc e61f847e74fd
src/mem/ruby/network/SConscript e61f847e74fd
src/mem/ruby/network/simple/SimpleNetwork.cc e61f847e74fd
src/mem/ruby/network/simple/Switch.cc e61f847e74fd
src/mem/ruby/slicc_interface/AbstractController.hh e61f847e74fd
src/mem/ruby/slicc_interface/AbstractController.cc e61f847e74fd
src/mem/slicc/symbols/StateMachine.py e61f847e74fd
src/python/swig/pyobject.cc e61f847e74fd
Diff: http://reviews.gem5.org/r/2845/diff/
Testing
-------
Regression tests with all Ruby protocols. Protocol file changes are in
separate patch (http://reviews.gem5.org/r/2844/) to keep this one slim.
Thanks,
Joel Hestness
Joel Hestness
2015-05-26 19:17:04 UTC
Permalink
Post by Joel Hestness
Post by Nilay Vaish
I went through both the patches. Unlikely that I am going to agree
these changes since we are dropping the functionality of connecting buffers
in python.
Thanks for reviewing this quickly. Before I spend the time to address the rest of your changes, I'd prefer to know more about why we're "connecting" buffers to ports in Python code. Connecting buffers to ports in Python is very confusing (buffers do not expose a port interface), and the code in pyobject.cc doesn't actually do anything with the port to which the buffer is connected. Can you elaborate why we do that and why it shouldn't be dropped?


- Joel


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2845/#review6397
-----------------------------------------------------------
Post by Joel Hestness
-----------------------------------------------------------
http://reviews.gem5.org/r/2845/
-----------------------------------------------------------
(Updated May 25, 2015, 11:51 p.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 10850:0fc919c768b2
---------------------------
ruby: Expose MessageBuffers as SimObjects
Expose MessageBuffers from SLICC controllers as SimObjects that can be
1) First and foremost, it exposes MessageBuffers as SimObjects that can be
manipulated in Python code. This allows parameters to be set and checked in
Python code to avoid obfuscating parameters within protocol files. Further, now
as SimObjects, MessageBuffer parameters are printed to config output files as a
way to track parameters across simulations (e.g. buffer sizes)
2) Cleans up special-case code for responseFromMemory buffers, and aligns their
instantiation and use with mandatoryQueue buffers. These two special buffers
are the only MessageBuffers that are exposed to components outside of SLICC
controllers, and they're both slave ends of these buffers. They should be
exposed outside of SLICC in the same way, and this patch does it.
3) Removes the obfuscated port-like connection of MessageBuffers. Ports are
now connected in the init function of the generated controllers.
Diffs
-----
src/mem/protocol/RubySlicc_Defines.sm e61f847e74fd
src/mem/ruby/network/MessageBuffer.hh e61f847e74fd
src/mem/ruby/network/MessageBuffer.cc e61f847e74fd
src/mem/ruby/network/SConscript e61f847e74fd
src/mem/ruby/network/simple/SimpleNetwork.cc e61f847e74fd
src/mem/ruby/network/simple/Switch.cc e61f847e74fd
src/mem/ruby/slicc_interface/AbstractController.hh e61f847e74fd
src/mem/ruby/slicc_interface/AbstractController.cc e61f847e74fd
src/mem/slicc/symbols/StateMachine.py e61f847e74fd
src/python/swig/pyobject.cc e61f847e74fd
Diff: http://reviews.gem5.org/r/2845/diff/
Testing
-------
Regression tests with all Ruby protocols. Protocol file changes are in
separate patch (http://reviews.gem5.org/r/2844/) to keep this one slim.
Thanks,
Joel Hestness
Nilay Vaish
2015-05-26 19:28:52 UTC
Permalink
Post by Joel Hestness
Post by Nilay Vaish
I went through both the patches. Unlikely that I am going to agree
these changes since we are dropping the functionality of connecting buffers
in python.
Thanks for reviewing this quickly. Before I spend the time to address
the rest of your changes, I'd prefer to know more about why we're
"connecting" buffers to ports in Python code. Connecting buffers to
ports in Python is very confusing (buffers do not expose a port
interface), and the code in pyobject.cc doesn't actually do anything
with the port to which the buffer is connected. Can you elaborate why we
do that and why it shouldn't be dropped?
When connections are made in the code produced by SLICC, those connections
are not visible in config.ini and connections can only be between a
controller and the network object. The master / slave python port
interface that currently exists allows those connections to be visible in
config.ini and we can connect any message buffer to any other message
buffer and not just the buffers belonging to the network object.


--
Nilay
Joel Hestness
2015-05-27 16:43:59 UTC
Permalink
Post by Joel Hestness
Post by Nilay Vaish
I went through both the patches. Unlikely that I am going to agree
these changes since we are dropping the functionality of connecting buffers
in python.
Thanks for reviewing this quickly. Before I spend the time to address the rest of your changes, I'd prefer to know more about why we're "connecting" buffers to ports in Python code. Connecting buffers to ports in Python is very confusing (buffers do not expose a port interface), and the code in pyobject.cc doesn't actually do anything with the port to which the buffer is connected. Can you elaborate why we do that and why it shouldn't be dropped?
I would be easy to move buffer-to-port connections back into Python code to get them printed in config.ini files. I do feel like the existing way of doing this is confusing, because buffers do not expose a port interface, so I'll be looking for clearer ways to get the same effect. I strongly prefer Python configurability of buffers rather than allowing sizing in the .sm files. I'm willing to spend some time to make it happen.

Can you let me know if you support moving MessageBuffers to Python like this, or if you would prefer we do it another way?
Post by Joel Hestness
Post by Nilay Vaish
src/mem/ruby/slicc_interface/AbstractController.cc, line 70
<http://reviews.gem5.org/r/2845/diff/1/?file=45415#file45415line70>
Don't you need to set the receiver and the ordering property?
Code for this is now auto-generated to be consistent with other MessageBuffers (in the constructor and init() functions).
Post by Joel Hestness
Post by Nilay Vaish
src/mem/ruby/slicc_interface/AbstractController.cc, line 294
<http://reviews.gem5.org/r/2845/diff/1/?file=45415#file45415line294>
Do we not need to functionally write to the memory?
Code for this is now auto-generated to be consistent with other MessageBuffers (in functionalWriteBuffers(PacketPtr& pkt)).
Post by Joel Hestness
Post by Nilay Vaish
src/mem/ruby/network/simple/SimpleNetwork.cc, line 124
<http://reviews.gem5.org/r/2845/diff/1/?file=45412#file45412line124>
This is not right.
Yep, this is what I'll need to tackle next.


- Joel


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2845/#review6397
-----------------------------------------------------------
Post by Joel Hestness
-----------------------------------------------------------
http://reviews.gem5.org/r/2845/
-----------------------------------------------------------
(Updated May 25, 2015, 11:51 p.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 10850:0fc919c768b2
---------------------------
ruby: Expose MessageBuffers as SimObjects
Expose MessageBuffers from SLICC controllers as SimObjects that can be
1) First and foremost, it exposes MessageBuffers as SimObjects that can be
manipulated in Python code. This allows parameters to be set and checked in
Python code to avoid obfuscating parameters within protocol files. Further, now
as SimObjects, MessageBuffer parameters are printed to config output files as a
way to track parameters across simulations (e.g. buffer sizes)
2) Cleans up special-case code for responseFromMemory buffers, and aligns their
instantiation and use with mandatoryQueue buffers. These two special buffers
are the only MessageBuffers that are exposed to components outside of SLICC
controllers, and they're both slave ends of these buffers. They should be
exposed outside of SLICC in the same way, and this patch does it.
3) Removes the obfuscated port-like connection of MessageBuffers. Ports are
now connected in the init function of the generated controllers.
Diffs
-----
src/mem/protocol/RubySlicc_Defines.sm e61f847e74fd
src/mem/ruby/network/MessageBuffer.hh e61f847e74fd
src/mem/ruby/network/MessageBuffer.cc e61f847e74fd
src/mem/ruby/network/SConscript e61f847e74fd
src/mem/ruby/network/simple/SimpleNetwork.cc e61f847e74fd
src/mem/ruby/network/simple/Switch.cc e61f847e74fd
src/mem/ruby/slicc_interface/AbstractController.hh e61f847e74fd
src/mem/ruby/slicc_interface/AbstractController.cc e61f847e74fd
src/mem/slicc/symbols/StateMachine.py e61f847e74fd
src/python/swig/pyobject.cc e61f847e74fd
Diff: http://reviews.gem5.org/r/2845/diff/
Testing
-------
Regression tests with all Ruby protocols. Protocol file changes are in
separate patch (http://reviews.gem5.org/r/2844/) to keep this one slim.
Thanks,
Joel Hestness
Brad Beckmann
2015-05-27 00:22:08 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2845/#review6403
-----------------------------------------------------------


I just have two questions so far on this patch.

1. What does Nilay mean that the "we are dropping the functionality of connecting buffers in python"? It wasn't too long ago when we added that functionality rather than doing it in the controller constructors. Why is that functionality going away and how was that decision determined?
2. Joel was it your intention to post this patch without moving the MessageBuffers created within the network (not the endpoints) to python? Was that to get early feedback before doing the possibly harder job of dealing with those other MessageBuffers? Were you hoping that Mark Wilkening to pick this patch up and add that support?


Thanks


src/mem/ruby/network/simple/SimpleNetwork.cc (line 124)
<http://reviews.gem5.org/r/2845/#comment5507>

I believe Nilay's concern is that you are instantiating a SimObject in the C++ code. I could be wrong, but I believe all other SimObjects are instantiated by swig via the python configuration files. I believe the expected implementation would be to create all these MessageBuffers in the Network/Switch .py files.


- Brad Beckmann
Post by Joel Hestness
-----------------------------------------------------------
http://reviews.gem5.org/r/2845/
-----------------------------------------------------------
(Updated May 25, 2015, 11:51 p.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 10850:0fc919c768b2
---------------------------
ruby: Expose MessageBuffers as SimObjects
Expose MessageBuffers from SLICC controllers as SimObjects that can be
1) First and foremost, it exposes MessageBuffers as SimObjects that can be
manipulated in Python code. This allows parameters to be set and checked in
Python code to avoid obfuscating parameters within protocol files. Further, now
as SimObjects, MessageBuffer parameters are printed to config output files as a
way to track parameters across simulations (e.g. buffer sizes)
2) Cleans up special-case code for responseFromMemory buffers, and aligns their
instantiation and use with mandatoryQueue buffers. These two special buffers
are the only MessageBuffers that are exposed to components outside of SLICC
controllers, and they're both slave ends of these buffers. They should be
exposed outside of SLICC in the same way, and this patch does it.
3) Removes the obfuscated port-like connection of MessageBuffers. Ports are
now connected in the init function of the generated controllers.
Diffs
-----
src/mem/protocol/RubySlicc_Defines.sm e61f847e74fd
src/mem/ruby/network/MessageBuffer.hh e61f847e74fd
src/mem/ruby/network/MessageBuffer.cc e61f847e74fd
src/mem/ruby/network/SConscript e61f847e74fd
src/mem/ruby/network/simple/SimpleNetwork.cc e61f847e74fd
src/mem/ruby/network/simple/Switch.cc e61f847e74fd
src/mem/ruby/slicc_interface/AbstractController.hh e61f847e74fd
src/mem/ruby/slicc_interface/AbstractController.cc e61f847e74fd
src/mem/slicc/symbols/StateMachine.py e61f847e74fd
src/python/swig/pyobject.cc e61f847e74fd
Diff: http://reviews.gem5.org/r/2845/diff/
Testing
-------
Regression tests with all Ruby protocols. Protocol file changes are in
separate patch (http://reviews.gem5.org/r/2844/) to keep this one slim.
Thanks,
Joel Hestness
Nilay Vaish
2015-05-27 01:31:35 UTC
Permalink
Post by Joel Hestness
-----------------------------------------------------------
http://reviews.gem5.org/r/2845/#review6403
-----------------------------------------------------------
I just have two questions so far on this patch.
1. What does Nilay mean that the "we are dropping the functionality of
connecting buffers in python"? It wasn't too long ago when we added
that functionality rather than doing it in the controller
constructors. Why is that functionality going away and how was that
decision determined?
My reading of Joel's patch is that we would not be connecting controllers
in python. We would be moving back to connecting controller in code
generated by SLICC. I am not in favor of this change. I want to retain
the ability to connect controllers in python configuration files.


--
Nilay
Joel Hestness
2015-05-27 16:32:58 UTC
Permalink
Post by Joel Hestness
Post by Brad Beckmann
I just have two questions so far on this patch.
1. What does Nilay mean that the "we are dropping the functionality of connecting buffers in python"? It wasn't too long ago when we added that functionality rather than doing it in the controller constructors. Why is that functionality going away and how was that decision determined?
2. Joel was it your intention to post this patch without moving the MessageBuffers created within the network (not the endpoints) to python? Was that to get early feedback before doing the possibly harder job of dealing with those other MessageBuffers? Were you hoping that Mark Wilkening to pick this patch up and add that support?
Thanks
1. I just made a quick decision to remove the buffer connection code in pyobject.cc. It wasn't clear to me that it was actually doing anything, because it just called into C++ code, which is generated to connect the buffers. I see Nilay's point that the connections are printed in the config.ini. It would be easy to add this back (though I still feel it is convoluted to connect a buffer to a port). I'll be looking for ways to address this.
2. Yes, I created this patch quickly in an effort to get feedback and to hopefully start us toward handling buffers in Python code. I have not yet looked at what it would take to move the buffers from the Switch and SimpleNetwork into Python code. I still prefer that we not introduce buffer sizing inside cache controllers, and I'm hoping we can align on a Python-configurable way to do it.

I'm willing to invest time in Python-configurability of MessageBuffers, because it will very likely save me time later. Please let me know if you like the direction of this patch (exposing MessageBuffers in Python), or if you think we should do that another way.


- Joel


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2845/#review6403
-----------------------------------------------------------
Post by Joel Hestness
-----------------------------------------------------------
http://reviews.gem5.org/r/2845/
-----------------------------------------------------------
(Updated May 25, 2015, 11:51 p.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 10850:0fc919c768b2
---------------------------
ruby: Expose MessageBuffers as SimObjects
Expose MessageBuffers from SLICC controllers as SimObjects that can be
1) First and foremost, it exposes MessageBuffers as SimObjects that can be
manipulated in Python code. This allows parameters to be set and checked in
Python code to avoid obfuscating parameters within protocol files. Further, now
as SimObjects, MessageBuffer parameters are printed to config output files as a
way to track parameters across simulations (e.g. buffer sizes)
2) Cleans up special-case code for responseFromMemory buffers, and aligns their
instantiation and use with mandatoryQueue buffers. These two special buffers
are the only MessageBuffers that are exposed to components outside of SLICC
controllers, and they're both slave ends of these buffers. They should be
exposed outside of SLICC in the same way, and this patch does it.
3) Removes the obfuscated port-like connection of MessageBuffers. Ports are
now connected in the init function of the generated controllers.
Diffs
-----
src/mem/protocol/RubySlicc_Defines.sm e61f847e74fd
src/mem/ruby/network/MessageBuffer.hh e61f847e74fd
src/mem/ruby/network/MessageBuffer.cc e61f847e74fd
src/mem/ruby/network/SConscript e61f847e74fd
src/mem/ruby/network/simple/SimpleNetwork.cc e61f847e74fd
src/mem/ruby/network/simple/Switch.cc e61f847e74fd
src/mem/ruby/slicc_interface/AbstractController.hh e61f847e74fd
src/mem/ruby/slicc_interface/AbstractController.cc e61f847e74fd
src/mem/slicc/symbols/StateMachine.py e61f847e74fd
src/python/swig/pyobject.cc e61f847e74fd
Diff: http://reviews.gem5.org/r/2845/diff/
Testing
-------
Regression tests with all Ruby protocols. Protocol file changes are in
separate patch (http://reviews.gem5.org/r/2844/) to keep this one slim.
Thanks,
Joel Hestness
Nilay Vaish
2015-05-27 18:00:25 UTC
Permalink
Post by Joel Hestness
Post by Brad Beckmann
I just have two questions so far on this patch.
1. What does Nilay mean that the "we are dropping the functionality of connecting buffers in python"? It wasn't too long ago when we added that functionality rather than doing it in the controller constructors. Why is that functionality going away and how was that decision determined?
2. Joel was it your intention to post this patch without moving the MessageBuffers created within the network (not the endpoints) to python? Was that to get early feedback before doing the possibly harder job of dealing with those other MessageBuffers? Were you hoping that Mark Wilkening to pick this patch up and add that support?
Thanks
1. I just made a quick decision to remove the buffer connection code in pyobject.cc. It wasn't clear to me that it was actually doing anything, because it just called into C++ code, which is generated to connect the buffers. I see Nilay's point that the connections are printed in the config.ini. It would be easy to add this back (though I still feel it is convoluted to connect a buffer to a port). I'll be looking for ways to address this.
2. Yes, I created this patch quickly in an effort to get feedback and to hopefully start us toward handling buffers in Python code. I have not yet looked at what it would take to move the buffers from the Switch and SimpleNetwork into Python code. I still prefer that we not introduce buffer sizing inside cache controllers, and I'm hoping we can align on a Python-configurable way to do it.
I'm willing to invest time in Python-configurability of MessageBuffers, because it will very likely save me time later. Please let me know if you like the direction of this patch (exposing MessageBuffers in Python), or if you think we should do that another way.
Just realized that MessageBuffer.py file is missing from the posted patch.

--
Nilay
Joel Hestness
2015-06-01 16:26:07 UTC
Permalink
Post by Nilay Vaish
src/mem/ruby/network/simple/SimpleNetwork.cc, line 124
<http://reviews.gem5.org/r/2845/diff/1/?file=45412#file45412line124>
I believe Nilay's concern is that you are instantiating a SimObject in the C++ code. I could be wrong, but I believe all other SimObjects are instantiated by swig via the python configuration files. I believe the expected implementation would be to create all these MessageBuffers in the Network/Switch .py files.
Right now this patch is missing the main point -- "First and foremost, it exposes MessageBuffers as SimObjects that can be manipulated in Python code. This allows parameters to be set and checked in Python code to avoid obfuscating parameters within protocol files." By instantiating MessageBuffers in C++ code with default parameters we cannot configure them individually at all. I think the difficult task here is being able to define all the controller defined MessageBuffers in python and set the MessageBuffers properly from the Params in the controller C++. I think we might need to autogenerate protocol specific python controller class definitions and params, give them defaults but allow network or topology python code to configure the protocol specific buffers.
I'm not sure I follow what you're saying. This patch *does* expose all appropriate MessageBuffers to be manipulated in Python for all generated controllers. When swig instantiates the MessageBuffers for the Python SimObjects, their parameters can be set up immediately in the MessageBuffer constructor without any need for code generation.

For this patch, I've collected the following feedback, and I plan to try to address them with further updates to this patch:
1) From Nilay: If we are going to expose MessageBuffer parameters in Python, then actually set parameters in the MessageBuffer constructor using Python parameters. This should be easy.
2) From Nilay: We need the port connections to be printed to the config.ini. This patch should not drop that. This should also be easy given that I had a version of this patch that left the port connections in at one point.
3) From Nilay and Brad: Move SimpleNetwork and Switch MessageBuffer instantiation into Python, since we should disallow C++ instantiation of SimObjects. I still have not had a chance to look at what this will take.


- Joel


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2845/#review6403
-----------------------------------------------------------
-----------------------------------------------------------
http://reviews.gem5.org/r/2845/
-----------------------------------------------------------
(Updated May 27, 2015, 6:13 p.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 10850:0c31c4ef98c0
---------------------------
ruby: Expose MessageBuffers as SimObjects
Expose MessageBuffers from SLICC controllers as SimObjects that can be
1) First and foremost, it exposes MessageBuffers as SimObjects that can be
manipulated in Python code. This allows parameters to be set and checked in
Python code to avoid obfuscating parameters within protocol files. Further, now
as SimObjects, MessageBuffer parameters are printed to config output files as a
way to track parameters across simulations (e.g. buffer sizes)
2) Cleans up special-case code for responseFromMemory buffers, and aligns their
instantiation and use with mandatoryQueue buffers. These two special buffers
are the only MessageBuffers that are exposed to components outside of SLICC
controllers, and they're both slave ends of these buffers. They should be
exposed outside of SLICC in the same way, and this patch does it.
3) Removes the obfuscated port-like connection of MessageBuffers. Ports are
now connected in the init function of the generated controllers.
Diffs
-----
src/mem/protocol/RubySlicc_Defines.sm e61f847e74fd
src/mem/ruby/network/MessageBuffer.hh e61f847e74fd
src/mem/ruby/network/MessageBuffer.cc e61f847e74fd
src/mem/ruby/network/MessageBuffer.py PRE-CREATION
src/mem/ruby/network/SConscript e61f847e74fd
src/mem/ruby/network/simple/SimpleNetwork.cc e61f847e74fd
src/mem/ruby/network/simple/Switch.cc e61f847e74fd
src/mem/ruby/slicc_interface/AbstractController.hh e61f847e74fd
src/mem/ruby/slicc_interface/AbstractController.cc e61f847e74fd
src/mem/slicc/symbols/StateMachine.py e61f847e74fd
src/python/swig/pyobject.cc e61f847e74fd
Diff: http://reviews.gem5.org/r/2845/diff/
Testing
-------
Regression tests with all Ruby protocols. Protocol file changes are in
separate patch (http://reviews.gem5.org/r/2844/) to keep this one slim.
Thanks,
Joel Hestness
Brad Beckmann
2015-06-01 17:23:06 UTC
Permalink
Post by Nilay Vaish
src/mem/ruby/network/simple/SimpleNetwork.cc, line 124
<http://reviews.gem5.org/r/2845/diff/1/?file=45412#file45412line124>
I believe Nilay's concern is that you are instantiating a SimObject in the C++ code. I could be wrong, but I believe all other SimObjects are instantiated by swig via the python configuration files. I believe the expected implementation would be to create all these MessageBuffers in the Network/Switch .py files.
Right now this patch is missing the main point -- "First and foremost, it exposes MessageBuffers as SimObjects that can be manipulated in Python code. This allows parameters to be set and checked in Python code to avoid obfuscating parameters within protocol files." By instantiating MessageBuffers in C++ code with default parameters we cannot configure them individually at all. I think the difficult task here is being able to define all the controller defined MessageBuffers in python and set the MessageBuffers properly from the Params in the controller C++. I think we might need to autogenerate protocol specific python controller class definitions and params, give them defaults but allow network or topology python code to configure the protocol specific buffers.
I'm not sure I follow what you're saying. This patch *does* expose all appropriate MessageBuffers to be manipulated in Python for all generated controllers. When swig instantiates the MessageBuffers for the Python SimObjects, their parameters can be set up immediately in the MessageBuffer constructor without any need for code generation.
1) From Nilay: If we are going to expose MessageBuffer parameters in Python, then actually set parameters in the MessageBuffer constructor using Python parameters. This should be easy.
2) From Nilay: We need the port connections to be printed to the config.ini. This patch should not drop that. This should also be easy given that I had a version of this patch that left the port connections in at one point.
3) From Nilay and Brad: Move SimpleNetwork and Switch MessageBuffer instantiation into Python, since we should disallow C++ instantiation of SimObjects. I still have not had a chance to look at what this will take.
Joel, I believe part of Mark's question will be answered when you take care of point #3 and remove the C++ instantiation of the Network MessageBuffers. I also believe Mark would like to see the Controller MessageBuffers configured differently than the Network MessageBuffers. That is one of the benefits of the patch http://reviews.gem5.org/r/2814/. I think it makes sense to merge those changes together because we want the network injector buffering to be sized differently that the internal network buffering.


- Brad


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2845/#review6403
-----------------------------------------------------------
-----------------------------------------------------------
http://reviews.gem5.org/r/2845/
-----------------------------------------------------------
(Updated May 27, 2015, 6:13 p.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 10850:0c31c4ef98c0
---------------------------
ruby: Expose MessageBuffers as SimObjects
Expose MessageBuffers from SLICC controllers as SimObjects that can be
1) First and foremost, it exposes MessageBuffers as SimObjects that can be
manipulated in Python code. This allows parameters to be set and checked in
Python code to avoid obfuscating parameters within protocol files. Further, now
as SimObjects, MessageBuffer parameters are printed to config output files as a
way to track parameters across simulations (e.g. buffer sizes)
2) Cleans up special-case code for responseFromMemory buffers, and aligns their
instantiation and use with mandatoryQueue buffers. These two special buffers
are the only MessageBuffers that are exposed to components outside of SLICC
controllers, and they're both slave ends of these buffers. They should be
exposed outside of SLICC in the same way, and this patch does it.
3) Removes the obfuscated port-like connection of MessageBuffers. Ports are
now connected in the init function of the generated controllers.
Diffs
-----
src/mem/protocol/RubySlicc_Defines.sm e61f847e74fd
src/mem/ruby/network/MessageBuffer.hh e61f847e74fd
src/mem/ruby/network/MessageBuffer.cc e61f847e74fd
src/mem/ruby/network/MessageBuffer.py PRE-CREATION
src/mem/ruby/network/SConscript e61f847e74fd
src/mem/ruby/network/simple/SimpleNetwork.cc e61f847e74fd
src/mem/ruby/network/simple/Switch.cc e61f847e74fd
src/mem/ruby/slicc_interface/AbstractController.hh e61f847e74fd
src/mem/ruby/slicc_interface/AbstractController.cc e61f847e74fd
src/mem/slicc/symbols/StateMachine.py e61f847e74fd
src/python/swig/pyobject.cc e61f847e74fd
Diff: http://reviews.gem5.org/r/2845/diff/
Testing
-------
Regression tests with all Ruby protocols. Protocol file changes are in
separate patch (http://reviews.gem5.org/r/2844/) to keep this one slim.
Thanks,
Joel Hestness
Joel Hestness
2015-06-01 17:32:26 UTC
Permalink
Post by Nilay Vaish
src/mem/ruby/network/simple/SimpleNetwork.cc, line 124
<http://reviews.gem5.org/r/2845/diff/1/?file=45412#file45412line124>
I believe Nilay's concern is that you are instantiating a SimObject in the C++ code. I could be wrong, but I believe all other SimObjects are instantiated by swig via the python configuration files. I believe the expected implementation would be to create all these MessageBuffers in the Network/Switch .py files.
Right now this patch is missing the main point -- "First and foremost, it exposes MessageBuffers as SimObjects that can be manipulated in Python code. This allows parameters to be set and checked in Python code to avoid obfuscating parameters within protocol files." By instantiating MessageBuffers in C++ code with default parameters we cannot configure them individually at all. I think the difficult task here is being able to define all the controller defined MessageBuffers in python and set the MessageBuffers properly from the Params in the controller C++. I think we might need to autogenerate protocol specific python controller class definitions and params, give them defaults but allow network or topology python code to configure the protocol specific buffers.
I'm not sure I follow what you're saying. This patch *does* expose all appropriate MessageBuffers to be manipulated in Python for all generated controllers. When swig instantiates the MessageBuffers for the Python SimObjects, their parameters can be set up immediately in the MessageBuffer constructor without any need for code generation.
1) From Nilay: If we are going to expose MessageBuffer parameters in Python, then actually set parameters in the MessageBuffer constructor using Python parameters. This should be easy.
2) From Nilay: We need the port connections to be printed to the config.ini. This patch should not drop that. This should also be easy given that I had a version of this patch that left the port connections in at one point.
3) From Nilay and Brad: Move SimpleNetwork and Switch MessageBuffer instantiation into Python, since we should disallow C++ instantiation of SimObjects. I still have not had a chance to look at what this will take.
Joel, I believe part of Mark's question will be answered when you take care of point #3 and remove the C++ instantiation of the Network MessageBuffers. I also believe Mark would like to see the Controller MessageBuffers configured differently than the Network MessageBuffers. That is one of the benefits of the patch http://reviews.gem5.org/r/2814/. I think it makes sense to merge those changes together because we want the network injector buffering to be sized differently that the internal network buffering.
Ok. Can you elaborate on what you mean by "Controller MessageBuffers configured differently than the Network MessageBuffers"? Differently in what ways? If all MessageBuffers are instantiated in Python, that should allow arbitrary configuration for either use, no?


- Joel


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2845/#review6403
-----------------------------------------------------------
-----------------------------------------------------------
http://reviews.gem5.org/r/2845/
-----------------------------------------------------------
(Updated May 27, 2015, 6:13 p.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 10850:0c31c4ef98c0
---------------------------
ruby: Expose MessageBuffers as SimObjects
Expose MessageBuffers from SLICC controllers as SimObjects that can be
1) First and foremost, it exposes MessageBuffers as SimObjects that can be
manipulated in Python code. This allows parameters to be set and checked in
Python code to avoid obfuscating parameters within protocol files. Further, now
as SimObjects, MessageBuffer parameters are printed to config output files as a
way to track parameters across simulations (e.g. buffer sizes)
2) Cleans up special-case code for responseFromMemory buffers, and aligns their
instantiation and use with mandatoryQueue buffers. These two special buffers
are the only MessageBuffers that are exposed to components outside of SLICC
controllers, and they're both slave ends of these buffers. They should be
exposed outside of SLICC in the same way, and this patch does it.
3) Removes the obfuscated port-like connection of MessageBuffers. Ports are
now connected in the init function of the generated controllers.
Diffs
-----
src/mem/protocol/RubySlicc_Defines.sm e61f847e74fd
src/mem/ruby/network/MessageBuffer.hh e61f847e74fd
src/mem/ruby/network/MessageBuffer.cc e61f847e74fd
src/mem/ruby/network/MessageBuffer.py PRE-CREATION
src/mem/ruby/network/SConscript e61f847e74fd
src/mem/ruby/network/simple/SimpleNetwork.cc e61f847e74fd
src/mem/ruby/network/simple/Switch.cc e61f847e74fd
src/mem/ruby/slicc_interface/AbstractController.hh e61f847e74fd
src/mem/ruby/slicc_interface/AbstractController.cc e61f847e74fd
src/mem/slicc/symbols/StateMachine.py e61f847e74fd
src/python/swig/pyobject.cc e61f847e74fd
Diff: http://reviews.gem5.org/r/2845/diff/
Testing
-------
Regression tests with all Ruby protocols. Protocol file changes are in
separate patch (http://reviews.gem5.org/r/2844/) to keep this one slim.
Thanks,
Joel Hestness
Brad Beckmann
2015-06-01 20:34:29 UTC
Permalink
Post by Nilay Vaish
src/mem/ruby/network/simple/SimpleNetwork.cc, line 124
<http://reviews.gem5.org/r/2845/diff/1/?file=45412#file45412line124>
I believe Nilay's concern is that you are instantiating a SimObject in the C++ code. I could be wrong, but I believe all other SimObjects are instantiated by swig via the python configuration files. I believe the expected implementation would be to create all these MessageBuffers in the Network/Switch .py files.
Right now this patch is missing the main point -- "First and foremost, it exposes MessageBuffers as SimObjects that can be manipulated in Python code. This allows parameters to be set and checked in Python code to avoid obfuscating parameters within protocol files." By instantiating MessageBuffers in C++ code with default parameters we cannot configure them individually at all. I think the difficult task here is being able to define all the controller defined MessageBuffers in python and set the MessageBuffers properly from the Params in the controller C++. I think we might need to autogenerate protocol specific python controller class definitions and params, give them defaults but allow network or topology python code to configure the protocol specific buffers.
I'm not sure I follow what you're saying. This patch *does* expose all appropriate MessageBuffers to be manipulated in Python for all generated controllers. When swig instantiates the MessageBuffers for the Python SimObjects, their parameters can be set up immediately in the MessageBuffer constructor without any need for code generation.
1) From Nilay: If we are going to expose MessageBuffer parameters in Python, then actually set parameters in the MessageBuffer constructor using Python parameters. This should be easy.
2) From Nilay: We need the port connections to be printed to the config.ini. This patch should not drop that. This should also be easy given that I had a version of this patch that left the port connections in at one point.
3) From Nilay and Brad: Move SimpleNetwork and Switch MessageBuffer instantiation into Python, since we should disallow C++ instantiation of SimObjects. I still have not had a chance to look at what this will take.
Joel, I believe part of Mark's question will be answered when you take care of point #3 and remove the C++ instantiation of the Network MessageBuffers. I also believe Mark would like to see the Controller MessageBuffers configured differently than the Network MessageBuffers. That is one of the benefits of the patch http://reviews.gem5.org/r/2814/. I think it makes sense to merge those changes together because we want the network injector buffering to be sized differently that the internal network buffering.
Ok. Can you elaborate on what you mean by "Controller MessageBuffers configured differently than the Network MessageBuffers"? Differently in what ways? If all MessageBuffers are instantiated in Python, that should allow arbitrary configuration for either use, no?
Sure, I should have been more clear. The key difference is that the Network MessageBuffers will be instantiated by the network Python code and it will be easy for the src/mem/ruby/network/*.py files to set the proper default sizes for these buffers. Meanwhile the controller Python code is autogenerated. Thus to set proper default values for these buffers we need to be able to specify them in the .sm files.


- Brad


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2845/#review6403
-----------------------------------------------------------
-----------------------------------------------------------
http://reviews.gem5.org/r/2845/
-----------------------------------------------------------
(Updated May 27, 2015, 6:13 p.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 10850:0c31c4ef98c0
---------------------------
ruby: Expose MessageBuffers as SimObjects
Expose MessageBuffers from SLICC controllers as SimObjects that can be
1) First and foremost, it exposes MessageBuffers as SimObjects that can be
manipulated in Python code. This allows parameters to be set and checked in
Python code to avoid obfuscating parameters within protocol files. Further, now
as SimObjects, MessageBuffer parameters are printed to config output files as a
way to track parameters across simulations (e.g. buffer sizes)
2) Cleans up special-case code for responseFromMemory buffers, and aligns their
instantiation and use with mandatoryQueue buffers. These two special buffers
are the only MessageBuffers that are exposed to components outside of SLICC
controllers, and they're both slave ends of these buffers. They should be
exposed outside of SLICC in the same way, and this patch does it.
3) Removes the obfuscated port-like connection of MessageBuffers. Ports are
now connected in the init function of the generated controllers.
Diffs
-----
src/mem/protocol/RubySlicc_Defines.sm e61f847e74fd
src/mem/ruby/network/MessageBuffer.hh e61f847e74fd
src/mem/ruby/network/MessageBuffer.cc e61f847e74fd
src/mem/ruby/network/MessageBuffer.py PRE-CREATION
src/mem/ruby/network/SConscript e61f847e74fd
src/mem/ruby/network/simple/SimpleNetwork.cc e61f847e74fd
src/mem/ruby/network/simple/Switch.cc e61f847e74fd
src/mem/ruby/slicc_interface/AbstractController.hh e61f847e74fd
src/mem/ruby/slicc_interface/AbstractController.cc e61f847e74fd
src/mem/slicc/symbols/StateMachine.py e61f847e74fd
src/python/swig/pyobject.cc e61f847e74fd
Diff: http://reviews.gem5.org/r/2845/diff/
Testing
-------
Regression tests with all Ruby protocols. Protocol file changes are in
separate patch (http://reviews.gem5.org/r/2844/) to keep this one slim.
Thanks,
Joel Hestness
Joel Hestness
2015-05-27 18:13:50 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2845/
-----------------------------------------------------------

(Updated May 27, 2015, 6:13 p.m.)


Review request for Default.


Repository: gem5


Description (updated)
-------

Changeset 10850:0c31c4ef98c0
---------------------------
ruby: Expose MessageBuffers as SimObjects

Expose MessageBuffers from SLICC controllers as SimObjects that can be
manipulated in Python. This patch has numerous benefits:
1) First and foremost, it exposes MessageBuffers as SimObjects that can be
manipulated in Python code. This allows parameters to be set and checked in
Python code to avoid obfuscating parameters within protocol files. Further, now
as SimObjects, MessageBuffer parameters are printed to config output files as a
way to track parameters across simulations (e.g. buffer sizes)
2) Cleans up special-case code for responseFromMemory buffers, and aligns their
instantiation and use with mandatoryQueue buffers. These two special buffers
are the only MessageBuffers that are exposed to components outside of SLICC
controllers, and they're both slave ends of these buffers. They should be
exposed outside of SLICC in the same way, and this patch does it.
3) Removes the obfuscated port-like connection of MessageBuffers. Ports are
now connected in the init function of the generated controllers.


Diffs (updated)
-----

src/mem/protocol/RubySlicc_Defines.sm e61f847e74fd
src/mem/ruby/network/MessageBuffer.hh e61f847e74fd
src/mem/ruby/network/MessageBuffer.cc e61f847e74fd
src/mem/ruby/network/MessageBuffer.py PRE-CREATION
src/mem/ruby/network/SConscript e61f847e74fd
src/mem/ruby/network/simple/SimpleNetwork.cc e61f847e74fd
src/mem/ruby/network/simple/Switch.cc e61f847e74fd
src/mem/ruby/slicc_interface/AbstractController.hh e61f847e74fd
src/mem/ruby/slicc_interface/AbstractController.cc e61f847e74fd
src/mem/slicc/symbols/StateMachine.py e61f847e74fd
src/python/swig/pyobject.cc e61f847e74fd

Diff: http://reviews.gem5.org/r/2845/diff/


Testing
-------

Regression tests with all Ruby protocols. Protocol file changes are in
separate patch (http://reviews.gem5.org/r/2844/) to keep this one slim.


Thanks,

Joel Hestness
Joel Hestness
2015-06-08 18:52:11 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2845/
-----------------------------------------------------------

(Updated June 8, 2015, 6:52 p.m.)


Review request for Default.


Changes
-------

Addressed initial reviews:
1) Moved MessageBuffer parameters out to Python object
2) Reverted MessageBuffer port connecting code
3) Connect SimpleNetwork MessageBuffers in Python


Repository: gem5


Description (updated)
-------

Changeset 10859:501d2095e6a2
---------------------------
ruby: Expose MessageBuffers as SimObjects

Expose MessageBuffers from SLICC controllers as SimObjects that can be
manipulated in Python. This patch has numerous benefits:
1) First and foremost, it exposes MessageBuffers as SimObjects that can be
manipulated in Python code. This allows parameters to be set and checked in
Python code to avoid obfuscating parameters within protocol files. Further, now
as SimObjects, MessageBuffer parameters are printed to config output files as a
way to track parameters across simulations (e.g. buffer sizes)

2) Cleans up special-case code for responseFromMemory buffers, and aligns their
instantiation and use with mandatoryQueue buffers. These two special buffers
are the only MessageBuffers that are exposed to components outside of SLICC
controllers, and they're both slave ends of these buffers. They should be
exposed outside of SLICC in the same way, and this patch does it.

3) Distinguishes buffer-specific parameters from buffer-to-network parameters.
Specifically, buffer size, randomization, ordering, recycle latency, and ports
are all specific to a MessageBuffer, while the virtual network ID and type are
intrinsics of how the buffer is connected to network ports. The former are
specified in the Python object, while the latter are specified in the
controller *.sm files. Unlike buffer-specific parameters, which may need to
change depending on the simulated system structure, buffer-to-network
parameters can be specified statically for most or all different simulated
systems.


Diffs (updated)
-----

src/python/swig/pyobject.cc d02b45a554b5
src/mem/ruby/SConscript d02b45a554b5
src/mem/ruby/network/MessageBuffer.hh d02b45a554b5
src/mem/ruby/network/MessageBuffer.cc d02b45a554b5
src/mem/ruby/network/MessageBuffer.py PRE-CREATION
src/mem/ruby/network/SConscript d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.hh d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.cc d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.py d02b45a554b5
src/mem/ruby/network/simple/Switch.hh d02b45a554b5
src/mem/ruby/network/simple/Switch.cc d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.hh d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.cc d02b45a554b5
src/mem/slicc/ast/ObjDeclAST.py d02b45a554b5
src/mem/slicc/symbols/StateMachine.py d02b45a554b5
configs/ruby/Ruby.py d02b45a554b5
src/mem/protocol/RubySlicc_Defines.sm d02b45a554b5

Diff: http://reviews.gem5.org/r/2845/diff/


Testing
-------

Regression tests with all Ruby protocols. Protocol file changes are in
separate patch (http://reviews.gem5.org/r/2844/) to keep this one slim.


Thanks,

Joel Hestness
Joel Hestness
2015-06-20 16:14:40 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2845/
-----------------------------------------------------------

(Updated June 20, 2015, 4:14 p.m.)


Review request for Default.


Changes
-------

Minor: variable naming

Any further comments?


Repository: gem5


Description (updated)
-------

Changeset 10859:d0dddb8fc9de
---------------------------
ruby: Expose MessageBuffers as SimObjects

Expose MessageBuffers from SLICC controllers as SimObjects that can be
manipulated in Python. This patch has numerous benefits:
1) First and foremost, it exposes MessageBuffers as SimObjects that can be
manipulated in Python code. This allows parameters to be set and checked in
Python code to avoid obfuscating parameters within protocol files. Further, now
as SimObjects, MessageBuffer parameters are printed to config output files as a
way to track parameters across simulations (e.g. buffer sizes)

2) Cleans up special-case code for responseFromMemory buffers, and aligns their
instantiation and use with mandatoryQueue buffers. These two special buffers
are the only MessageBuffers that are exposed to components outside of SLICC
controllers, and they're both slave ends of these buffers. They should be
exposed outside of SLICC in the same way, and this patch does it.

3) Distinguishes buffer-specific parameters from buffer-to-network parameters.
Specifically, buffer size, randomization, ordering, recycle latency, and ports
are all specific to a MessageBuffer, while the virtual network ID and type are
intrinsics of how the buffer is connected to network ports. The former are
specified in the Python object, while the latter are specified in the
controller *.sm files. Unlike buffer-specific parameters, which may need to
change depending on the simulated system structure, buffer-to-network
parameters can be specified statically for most or all different simulated
systems.


Diffs (updated)
-----

configs/ruby/Ruby.py d02b45a554b5
src/mem/protocol/RubySlicc_Defines.sm d02b45a554b5
src/mem/ruby/SConscript d02b45a554b5
src/mem/ruby/network/MessageBuffer.hh d02b45a554b5
src/mem/ruby/network/MessageBuffer.cc d02b45a554b5
src/mem/ruby/network/MessageBuffer.py PRE-CREATION
src/mem/ruby/network/SConscript d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.hh d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.cc d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.py d02b45a554b5
src/mem/ruby/network/simple/Switch.hh d02b45a554b5
src/mem/ruby/network/simple/Switch.cc d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.hh d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.cc d02b45a554b5
src/mem/slicc/ast/ObjDeclAST.py d02b45a554b5
src/mem/slicc/symbols/StateMachine.py d02b45a554b5
src/python/swig/pyobject.cc d02b45a554b5

Diff: http://reviews.gem5.org/r/2845/diff/


Testing
-------

Regression tests with all Ruby protocols. Protocol file changes are in
separate patch (http://reviews.gem5.org/r/2844/) to keep this one slim.


Thanks,

Joel Hestness
Brad Beckmann
2015-06-22 17:06:36 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2845/#review6548
-----------------------------------------------------------



configs/ruby/Ruby.py (line 98)
<http://reviews.gem5.org/r/2845/#comment5641>

Overall, can this code be moved into the Switch and Liny .py files rather than have it here in Ruby.py?

Also can we avoid setting the recycle_latency variable and just specify the default in the MessageBuffer.py file?

Finally, please correct me if I'm wrong, but I believe that all message buffers within the network have always been ordered, correct? If so, this patch maintains that same functionality, which is great. It would be good to add a comment pointing that out.



src/mem/ruby/network/MessageBuffer.py (line 39)
<http://reviews.gem5.org/r/2845/#comment5640>

Why is a default recycle latency not set?

Why is this "Parent.recycle_latency"? Does both swtiches and controllers define a recycle latency? It doesn't appear so with this patch. I'm a bit confused how this actually works. I do not believe switches will ever use recycles, so it doesn't make much sense to define this default in the switch parent. Also many protocols avoid using recycles. Those protocols should not need to specify a recycle latency either.


- Brad Beckmann
Post by Joel Hestness
-----------------------------------------------------------
http://reviews.gem5.org/r/2845/
-----------------------------------------------------------
(Updated June 20, 2015, 4:14 p.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 10859:d0dddb8fc9de
---------------------------
ruby: Expose MessageBuffers as SimObjects
Expose MessageBuffers from SLICC controllers as SimObjects that can be
1) First and foremost, it exposes MessageBuffers as SimObjects that can be
manipulated in Python code. This allows parameters to be set and checked in
Python code to avoid obfuscating parameters within protocol files. Further, now
as SimObjects, MessageBuffer parameters are printed to config output files as a
way to track parameters across simulations (e.g. buffer sizes)
2) Cleans up special-case code for responseFromMemory buffers, and aligns their
instantiation and use with mandatoryQueue buffers. These two special buffers
are the only MessageBuffers that are exposed to components outside of SLICC
controllers, and they're both slave ends of these buffers. They should be
exposed outside of SLICC in the same way, and this patch does it.
3) Distinguishes buffer-specific parameters from buffer-to-network parameters.
Specifically, buffer size, randomization, ordering, recycle latency, and ports
are all specific to a MessageBuffer, while the virtual network ID and type are
intrinsics of how the buffer is connected to network ports. The former are
specified in the Python object, while the latter are specified in the
controller *.sm files. Unlike buffer-specific parameters, which may need to
change depending on the simulated system structure, buffer-to-network
parameters can be specified statically for most or all different simulated
systems.
Diffs
-----
configs/ruby/Ruby.py d02b45a554b5
src/mem/protocol/RubySlicc_Defines.sm d02b45a554b5
src/mem/ruby/SConscript d02b45a554b5
src/mem/ruby/network/MessageBuffer.hh d02b45a554b5
src/mem/ruby/network/MessageBuffer.cc d02b45a554b5
src/mem/ruby/network/MessageBuffer.py PRE-CREATION
src/mem/ruby/network/SConscript d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.hh d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.cc d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.py d02b45a554b5
src/mem/ruby/network/simple/Switch.hh d02b45a554b5
src/mem/ruby/network/simple/Switch.cc d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.hh d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.cc d02b45a554b5
src/mem/slicc/ast/ObjDeclAST.py d02b45a554b5
src/mem/slicc/symbols/StateMachine.py d02b45a554b5
src/python/swig/pyobject.cc d02b45a554b5
Diff: http://reviews.gem5.org/r/2845/diff/
Testing
-------
Regression tests with all Ruby protocols. Protocol file changes are in
separate patch (http://reviews.gem5.org/r/2844/) to keep this one slim.
Thanks,
Joel Hestness
Joel Hestness
2015-06-24 16:54:13 UTC
Permalink
Post by Joel Hestness
configs/ruby/Ruby.py, line 98
<http://reviews.gem5.org/r/2845/diff/4/?file=46640#file46640line98>
Overall, can this code be moved into the Switch and Liny .py files rather than have it here in Ruby.py?
Also can we avoid setting the recycle_latency variable and just specify the default in the MessageBuffer.py file?
Finally, please correct me if I'm wrong, but I believe that all message buffers within the network have always been ordered, correct? If so, this patch maintains that same functionality, which is great. It would be good to add a comment pointing that out.
I'm splitting these comments into separate issues for easier tracking. Since these affect different portions of code, please open them as separate issues going forward.


- Joel


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2845/#review6548
-----------------------------------------------------------
Post by Joel Hestness
-----------------------------------------------------------
http://reviews.gem5.org/r/2845/
-----------------------------------------------------------
(Updated June 20, 2015, 4:14 p.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 10859:d0dddb8fc9de
---------------------------
ruby: Expose MessageBuffers as SimObjects
Expose MessageBuffers from SLICC controllers as SimObjects that can be
1) First and foremost, it exposes MessageBuffers as SimObjects that can be
manipulated in Python code. This allows parameters to be set and checked in
Python code to avoid obfuscating parameters within protocol files. Further, now
as SimObjects, MessageBuffer parameters are printed to config output files as a
way to track parameters across simulations (e.g. buffer sizes)
2) Cleans up special-case code for responseFromMemory buffers, and aligns their
instantiation and use with mandatoryQueue buffers. These two special buffers
are the only MessageBuffers that are exposed to components outside of SLICC
controllers, and they're both slave ends of these buffers. They should be
exposed outside of SLICC in the same way, and this patch does it.
3) Distinguishes buffer-specific parameters from buffer-to-network parameters.
Specifically, buffer size, randomization, ordering, recycle latency, and ports
are all specific to a MessageBuffer, while the virtual network ID and type are
intrinsics of how the buffer is connected to network ports. The former are
specified in the Python object, while the latter are specified in the
controller *.sm files. Unlike buffer-specific parameters, which may need to
change depending on the simulated system structure, buffer-to-network
parameters can be specified statically for most or all different simulated
systems.
Diffs
-----
configs/ruby/Ruby.py d02b45a554b5
src/mem/protocol/RubySlicc_Defines.sm d02b45a554b5
src/mem/ruby/SConscript d02b45a554b5
src/mem/ruby/network/MessageBuffer.hh d02b45a554b5
src/mem/ruby/network/MessageBuffer.cc d02b45a554b5
src/mem/ruby/network/MessageBuffer.py PRE-CREATION
src/mem/ruby/network/SConscript d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.hh d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.cc d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.py d02b45a554b5
src/mem/ruby/network/simple/Switch.hh d02b45a554b5
src/mem/ruby/network/simple/Switch.cc d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.hh d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.cc d02b45a554b5
src/mem/slicc/ast/ObjDeclAST.py d02b45a554b5
src/mem/slicc/symbols/StateMachine.py d02b45a554b5
src/python/swig/pyobject.cc d02b45a554b5
Diff: http://reviews.gem5.org/r/2845/diff/
Testing
-------
Regression tests with all Ruby protocols. Protocol file changes are in
separate patch (http://reviews.gem5.org/r/2844/) to keep this one slim.
Thanks,
Joel Hestness
Joel Hestness
2015-06-24 17:28:52 UTC
Permalink
Post by Joel Hestness
src/mem/ruby/network/MessageBuffer.py, line 39
<http://reviews.gem5.org/r/2845/diff/4/?file=46645#file46645line39>
Why is a default recycle latency not set?
Why is this "Parent.recycle_latency"? Does both swtiches and controllers define a recycle latency? It doesn't appear so with this patch. I'm a bit confused how this actually works. I do not believe switches will ever use recycles, so it doesn't make much sense to define this default in the switch parent. Also many protocols avoid using recycles. Those protocols should not need to specify a recycle latency either.
Here again, I just copied the convention that was used when these were in SLICC. Specifically, AbstractControllers require a recycle_latency to be defined, and it was used to set their MessageBuffer recycle_latency (you can see this in the SLICC code I removed in this patch). MessageBuffers in the network never set a default (was probably a bad idea).

Here are the options for setting recycle_latency:
1) Define a default of the parent's recycle_latency parameter
2) Define a default of 0

If we go with (2), there wouldn't be a way to set a single recycle_latency for all of a controller's MessageBuffers. We'd need to explicitly set them all when non-zero. This sounds like it would result in tons of latent bugs.

I think (1) is best: For MessageBuffers within controllers, recycle_latency will at least be set with the controller parent's value, and the user can explictly override this value by setting any particular buffer's recycle_latency in the config files. Network MessageBuffers should explicitly set recycle_latency to indicate that there is no latency.

A possibility to address your desire would be to add, in SimpleNetwork and SimpleRouter, a recycle_latency parameter that defaults to 0. That way, the MessageBuffers would inherit these values and could still be overridden by a user. Would that be agreeable?


- Joel


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2845/#review6548
-----------------------------------------------------------
Post by Joel Hestness
-----------------------------------------------------------
http://reviews.gem5.org/r/2845/
-----------------------------------------------------------
(Updated June 20, 2015, 4:14 p.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 10859:d0dddb8fc9de
---------------------------
ruby: Expose MessageBuffers as SimObjects
Expose MessageBuffers from SLICC controllers as SimObjects that can be
1) First and foremost, it exposes MessageBuffers as SimObjects that can be
manipulated in Python code. This allows parameters to be set and checked in
Python code to avoid obfuscating parameters within protocol files. Further, now
as SimObjects, MessageBuffer parameters are printed to config output files as a
way to track parameters across simulations (e.g. buffer sizes)
2) Cleans up special-case code for responseFromMemory buffers, and aligns their
instantiation and use with mandatoryQueue buffers. These two special buffers
are the only MessageBuffers that are exposed to components outside of SLICC
controllers, and they're both slave ends of these buffers. They should be
exposed outside of SLICC in the same way, and this patch does it.
3) Distinguishes buffer-specific parameters from buffer-to-network parameters.
Specifically, buffer size, randomization, ordering, recycle latency, and ports
are all specific to a MessageBuffer, while the virtual network ID and type are
intrinsics of how the buffer is connected to network ports. The former are
specified in the Python object, while the latter are specified in the
controller *.sm files. Unlike buffer-specific parameters, which may need to
change depending on the simulated system structure, buffer-to-network
parameters can be specified statically for most or all different simulated
systems.
Diffs
-----
configs/ruby/Ruby.py d02b45a554b5
src/mem/protocol/RubySlicc_Defines.sm d02b45a554b5
src/mem/ruby/SConscript d02b45a554b5
src/mem/ruby/network/MessageBuffer.hh d02b45a554b5
src/mem/ruby/network/MessageBuffer.cc d02b45a554b5
src/mem/ruby/network/MessageBuffer.py PRE-CREATION
src/mem/ruby/network/SConscript d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.hh d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.cc d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.py d02b45a554b5
src/mem/ruby/network/simple/Switch.hh d02b45a554b5
src/mem/ruby/network/simple/Switch.cc d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.hh d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.cc d02b45a554b5
src/mem/slicc/ast/ObjDeclAST.py d02b45a554b5
src/mem/slicc/symbols/StateMachine.py d02b45a554b5
src/python/swig/pyobject.cc d02b45a554b5
Diff: http://reviews.gem5.org/r/2845/diff/
Testing
-------
Regression tests with all Ruby protocols. Protocol file changes are in
separate patch (http://reviews.gem5.org/r/2844/) to keep this one slim.
Thanks,
Joel Hestness
Brad Beckmann
2015-06-26 15:48:21 UTC
Permalink
Post by Joel Hestness
src/mem/ruby/network/MessageBuffer.py, line 39
<http://reviews.gem5.org/r/2845/diff/4/?file=46645#file46645line39>
Why is a default recycle latency not set?
Why is this "Parent.recycle_latency"? Does both swtiches and controllers define a recycle latency? It doesn't appear so with this patch. I'm a bit confused how this actually works. I do not believe switches will ever use recycles, so it doesn't make much sense to define this default in the switch parent. Also many protocols avoid using recycles. Those protocols should not need to specify a recycle latency either.
Here again, I just copied the convention that was used when these were in SLICC. Specifically, AbstractControllers require a recycle_latency to be defined, and it was used to set their MessageBuffer recycle_latency (you can see this in the SLICC code I removed in this patch). MessageBuffers in the network never set a default (was probably a bad idea).
1) Define a default of the parent's recycle_latency parameter
2) Define a default of 0
If we go with (2), there wouldn't be a way to set a single recycle_latency for all of a controller's MessageBuffers. We'd need to explicitly set them all when non-zero. This sounds like it would result in tons of latent bugs.
I think (1) is best: For MessageBuffers within controllers, recycle_latency will at least be set with the controller parent's value, and the user can explictly override this value by setting any particular buffer's recycle_latency in the config files. Network MessageBuffers should explicitly set recycle_latency to indicate that there is no latency.
A possibility to address your desire would be to add, in SimpleNetwork and SimpleRouter, a recycle_latency parameter that defaults to 0. That way, the MessageBuffers would inherit these values and could still be overridden by a user. Would that be agreeable?
It sounds like we agree that recycling and setting a network recycle latency does not make much sense for network MessageBuffers. Since the network never sets a default recycle_latency, I'm still confused how the current code even works. It seems like the SimpleNetwork and SimpleRouter must set a recycle_latency parameter that defaults to 0.

Recycle latency is really a controller feature. I wonder if the best way to resolve this confusion is to remove the MessageBuffer recycle latency parameter and just have the controllers pass their recycle latency to the recycle() function.


- Brad


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2845/#review6548
-----------------------------------------------------------
Post by Joel Hestness
-----------------------------------------------------------
http://reviews.gem5.org/r/2845/
-----------------------------------------------------------
(Updated June 20, 2015, 4:14 p.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 10859:d0dddb8fc9de
---------------------------
ruby: Expose MessageBuffers as SimObjects
Expose MessageBuffers from SLICC controllers as SimObjects that can be
1) First and foremost, it exposes MessageBuffers as SimObjects that can be
manipulated in Python code. This allows parameters to be set and checked in
Python code to avoid obfuscating parameters within protocol files. Further, now
as SimObjects, MessageBuffer parameters are printed to config output files as a
way to track parameters across simulations (e.g. buffer sizes)
2) Cleans up special-case code for responseFromMemory buffers, and aligns their
instantiation and use with mandatoryQueue buffers. These two special buffers
are the only MessageBuffers that are exposed to components outside of SLICC
controllers, and they're both slave ends of these buffers. They should be
exposed outside of SLICC in the same way, and this patch does it.
3) Distinguishes buffer-specific parameters from buffer-to-network parameters.
Specifically, buffer size, randomization, ordering, recycle latency, and ports
are all specific to a MessageBuffer, while the virtual network ID and type are
intrinsics of how the buffer is connected to network ports. The former are
specified in the Python object, while the latter are specified in the
controller *.sm files. Unlike buffer-specific parameters, which may need to
change depending on the simulated system structure, buffer-to-network
parameters can be specified statically for most or all different simulated
systems.
Diffs
-----
configs/ruby/Ruby.py d02b45a554b5
src/mem/protocol/RubySlicc_Defines.sm d02b45a554b5
src/mem/ruby/SConscript d02b45a554b5
src/mem/ruby/network/MessageBuffer.hh d02b45a554b5
src/mem/ruby/network/MessageBuffer.cc d02b45a554b5
src/mem/ruby/network/MessageBuffer.py PRE-CREATION
src/mem/ruby/network/SConscript d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.hh d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.cc d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.py d02b45a554b5
src/mem/ruby/network/simple/Switch.hh d02b45a554b5
src/mem/ruby/network/simple/Switch.cc d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.hh d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.cc d02b45a554b5
src/mem/slicc/ast/ObjDeclAST.py d02b45a554b5
src/mem/slicc/symbols/StateMachine.py d02b45a554b5
src/python/swig/pyobject.cc d02b45a554b5
Diff: http://reviews.gem5.org/r/2845/diff/
Testing
-------
Regression tests with all Ruby protocols. Protocol file changes are in
separate patch (http://reviews.gem5.org/r/2844/) to keep this one slim.
Thanks,
Joel Hestness
Joel Hestness
2015-06-26 16:42:16 UTC
Permalink
Post by Joel Hestness
src/mem/ruby/network/MessageBuffer.py, line 39
<http://reviews.gem5.org/r/2845/diff/4/?file=46645#file46645line39>
Why is a default recycle latency not set?
Why is this "Parent.recycle_latency"? Does both swtiches and controllers define a recycle latency? It doesn't appear so with this patch. I'm a bit confused how this actually works. I do not believe switches will ever use recycles, so it doesn't make much sense to define this default in the switch parent. Also many protocols avoid using recycles. Those protocols should not need to specify a recycle latency either.
Here again, I just copied the convention that was used when these were in SLICC. Specifically, AbstractControllers require a recycle_latency to be defined, and it was used to set their MessageBuffer recycle_latency (you can see this in the SLICC code I removed in this patch). MessageBuffers in the network never set a default (was probably a bad idea).
1) Define a default of the parent's recycle_latency parameter
2) Define a default of 0
If we go with (2), there wouldn't be a way to set a single recycle_latency for all of a controller's MessageBuffers. We'd need to explicitly set them all when non-zero. This sounds like it would result in tons of latent bugs.
I think (1) is best: For MessageBuffers within controllers, recycle_latency will at least be set with the controller parent's value, and the user can explictly override this value by setting any particular buffer's recycle_latency in the config files. Network MessageBuffers should explicitly set recycle_latency to indicate that there is no latency.
A possibility to address your desire would be to add, in SimpleNetwork and SimpleRouter, a recycle_latency parameter that defaults to 0. That way, the MessageBuffers would inherit these values and could still be overridden by a user. Would that be agreeable?
It sounds like we agree that recycling and setting a network recycle latency does not make much sense for network MessageBuffers. Since the network never sets a default recycle_latency, I'm still confused how the current code even works. It seems like the SimpleNetwork and SimpleRouter must set a recycle_latency parameter that defaults to 0.
Recycle latency is really a controller feature. I wonder if the best way to resolve this confusion is to remove the MessageBuffer recycle latency parameter and just have the controllers pass their recycle latency to the recycle() function.
Since the network never sets a default recycle_latency, I'm still confused how the current code even works. It seems like the SimpleNetwork and SimpleRouter must set a recycle_latency parameter that defaults to 0.
This patch explicitly specifies recycle_latency=0 for all network/switch MessageBuffers during their instantiation (this is precisely what you took issue with in your other review...). Neither SimpleNetwork or SimpleRouter have a recycle_latency parameter.
Post by Joel Hestness
Recycle latency is really a controller feature. I wonder if the best way to resolve this confusion is to remove the MessageBuffer recycle latency parameter and just have the controllers pass their recycle latency to the recycle() function.
Maybe so, but this is outside the scope of this patch, which just aims to promote MessageBuffers to SimObjects. The MessageBuffer's recycle_latency variable is the same as it was previously, and this patch improves its setting by explicitly setting the value to 0 for network buffers (which was not happening previously). If we want to remove this variable from MessageBuffer, I think it should be done in a separate patch.


- Joel


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2845/#review6548
-----------------------------------------------------------
Post by Joel Hestness
-----------------------------------------------------------
http://reviews.gem5.org/r/2845/
-----------------------------------------------------------
(Updated June 20, 2015, 4:14 p.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 10859:d0dddb8fc9de
---------------------------
ruby: Expose MessageBuffers as SimObjects
Expose MessageBuffers from SLICC controllers as SimObjects that can be
1) First and foremost, it exposes MessageBuffers as SimObjects that can be
manipulated in Python code. This allows parameters to be set and checked in
Python code to avoid obfuscating parameters within protocol files. Further, now
as SimObjects, MessageBuffer parameters are printed to config output files as a
way to track parameters across simulations (e.g. buffer sizes)
2) Cleans up special-case code for responseFromMemory buffers, and aligns their
instantiation and use with mandatoryQueue buffers. These two special buffers
are the only MessageBuffers that are exposed to components outside of SLICC
controllers, and they're both slave ends of these buffers. They should be
exposed outside of SLICC in the same way, and this patch does it.
3) Distinguishes buffer-specific parameters from buffer-to-network parameters.
Specifically, buffer size, randomization, ordering, recycle latency, and ports
are all specific to a MessageBuffer, while the virtual network ID and type are
intrinsics of how the buffer is connected to network ports. The former are
specified in the Python object, while the latter are specified in the
controller *.sm files. Unlike buffer-specific parameters, which may need to
change depending on the simulated system structure, buffer-to-network
parameters can be specified statically for most or all different simulated
systems.
Diffs
-----
configs/ruby/Ruby.py d02b45a554b5
src/mem/protocol/RubySlicc_Defines.sm d02b45a554b5
src/mem/ruby/SConscript d02b45a554b5
src/mem/ruby/network/MessageBuffer.hh d02b45a554b5
src/mem/ruby/network/MessageBuffer.cc d02b45a554b5
src/mem/ruby/network/MessageBuffer.py PRE-CREATION
src/mem/ruby/network/SConscript d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.hh d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.cc d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.py d02b45a554b5
src/mem/ruby/network/simple/Switch.hh d02b45a554b5
src/mem/ruby/network/simple/Switch.cc d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.hh d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.cc d02b45a554b5
src/mem/slicc/ast/ObjDeclAST.py d02b45a554b5
src/mem/slicc/symbols/StateMachine.py d02b45a554b5
src/python/swig/pyobject.cc d02b45a554b5
Diff: http://reviews.gem5.org/r/2845/diff/
Testing
-------
Regression tests with all Ruby protocols. Protocol file changes are in
separate patch (http://reviews.gem5.org/r/2844/) to keep this one slim.
Thanks,
Joel Hestness
Brad Beckmann
2015-06-26 17:15:40 UTC
Permalink
Post by Joel Hestness
src/mem/ruby/network/MessageBuffer.py, line 39
<http://reviews.gem5.org/r/2845/diff/4/?file=46645#file46645line39>
Why is a default recycle latency not set?
Why is this "Parent.recycle_latency"? Does both swtiches and controllers define a recycle latency? It doesn't appear so with this patch. I'm a bit confused how this actually works. I do not believe switches will ever use recycles, so it doesn't make much sense to define this default in the switch parent. Also many protocols avoid using recycles. Those protocols should not need to specify a recycle latency either.
Here again, I just copied the convention that was used when these were in SLICC. Specifically, AbstractControllers require a recycle_latency to be defined, and it was used to set their MessageBuffer recycle_latency (you can see this in the SLICC code I removed in this patch). MessageBuffers in the network never set a default (was probably a bad idea).
1) Define a default of the parent's recycle_latency parameter
2) Define a default of 0
If we go with (2), there wouldn't be a way to set a single recycle_latency for all of a controller's MessageBuffers. We'd need to explicitly set them all when non-zero. This sounds like it would result in tons of latent bugs.
I think (1) is best: For MessageBuffers within controllers, recycle_latency will at least be set with the controller parent's value, and the user can explictly override this value by setting any particular buffer's recycle_latency in the config files. Network MessageBuffers should explicitly set recycle_latency to indicate that there is no latency.
A possibility to address your desire would be to add, in SimpleNetwork and SimpleRouter, a recycle_latency parameter that defaults to 0. That way, the MessageBuffers would inherit these values and could still be overridden by a user. Would that be agreeable?
It sounds like we agree that recycling and setting a network recycle latency does not make much sense for network MessageBuffers. Since the network never sets a default recycle_latency, I'm still confused how the current code even works. It seems like the SimpleNetwork and SimpleRouter must set a recycle_latency parameter that defaults to 0.
Recycle latency is really a controller feature. I wonder if the best way to resolve this confusion is to remove the MessageBuffer recycle latency parameter and just have the controllers pass their recycle latency to the recycle() function.
Post by Brad Beckmann
Since the network never sets a default recycle_latency, I'm still confused how the current code even works. It seems like the SimpleNetwork and SimpleRouter must set a recycle_latency parameter that defaults to 0.
This patch explicitly specifies recycle_latency=0 for all network/switch MessageBuffers during their instantiation (this is precisely what you took issue with in your other review...). Neither SimpleNetwork or SimpleRouter have a recycle_latency parameter.
Post by Brad Beckmann
Recycle latency is really a controller feature. I wonder if the best way to resolve this confusion is to remove the MessageBuffer recycle latency parameter and just have the controllers pass their recycle latency to the recycle() function.
Maybe so, but this is outside the scope of this patch, which just aims to promote MessageBuffers to SimObjects. The MessageBuffer's recycle_latency variable is the same as it was previously, and this patch improves its setting by explicitly setting the value to 0 for network buffers (which was not happening previously). If we want to remove this variable from MessageBuffer, I think it should be done in a separate patch.
Network MessageBuffers did not set the value to 0 because it was never used. By making MessageBuffers SimObjects, you are changing how MessageBuffes are initiated. If you want to minimize the changes, then you should choose option (2) "define a default of 0" and make sure that the controllers overwrite the value. That is most similar to what is done before this patch. The parent recycle_latency trick is cool, but some parents (network/switches) don't have a recycle latency.

Since you want to do option (1) how about we compromize and say move loops to SimpleNetwork and SimpleLink.py and set the Network and Switch recycle latency to zero. In the Network and Switch add a comment that makes it clear that these parameters don't make much sense.


- Brad


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2845/#review6548
-----------------------------------------------------------
Post by Joel Hestness
-----------------------------------------------------------
http://reviews.gem5.org/r/2845/
-----------------------------------------------------------
(Updated June 20, 2015, 4:14 p.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 10859:d0dddb8fc9de
---------------------------
ruby: Expose MessageBuffers as SimObjects
Expose MessageBuffers from SLICC controllers as SimObjects that can be
1) First and foremost, it exposes MessageBuffers as SimObjects that can be
manipulated in Python code. This allows parameters to be set and checked in
Python code to avoid obfuscating parameters within protocol files. Further, now
as SimObjects, MessageBuffer parameters are printed to config output files as a
way to track parameters across simulations (e.g. buffer sizes)
2) Cleans up special-case code for responseFromMemory buffers, and aligns their
instantiation and use with mandatoryQueue buffers. These two special buffers
are the only MessageBuffers that are exposed to components outside of SLICC
controllers, and they're both slave ends of these buffers. They should be
exposed outside of SLICC in the same way, and this patch does it.
3) Distinguishes buffer-specific parameters from buffer-to-network parameters.
Specifically, buffer size, randomization, ordering, recycle latency, and ports
are all specific to a MessageBuffer, while the virtual network ID and type are
intrinsics of how the buffer is connected to network ports. The former are
specified in the Python object, while the latter are specified in the
controller *.sm files. Unlike buffer-specific parameters, which may need to
change depending on the simulated system structure, buffer-to-network
parameters can be specified statically for most or all different simulated
systems.
Diffs
-----
configs/ruby/Ruby.py d02b45a554b5
src/mem/protocol/RubySlicc_Defines.sm d02b45a554b5
src/mem/ruby/SConscript d02b45a554b5
src/mem/ruby/network/MessageBuffer.hh d02b45a554b5
src/mem/ruby/network/MessageBuffer.cc d02b45a554b5
src/mem/ruby/network/MessageBuffer.py PRE-CREATION
src/mem/ruby/network/SConscript d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.hh d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.cc d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.py d02b45a554b5
src/mem/ruby/network/simple/Switch.hh d02b45a554b5
src/mem/ruby/network/simple/Switch.cc d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.hh d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.cc d02b45a554b5
src/mem/slicc/ast/ObjDeclAST.py d02b45a554b5
src/mem/slicc/symbols/StateMachine.py d02b45a554b5
src/python/swig/pyobject.cc d02b45a554b5
Diff: http://reviews.gem5.org/r/2845/diff/
Testing
-------
Regression tests with all Ruby protocols. Protocol file changes are in
separate patch (http://reviews.gem5.org/r/2844/) to keep this one slim.
Thanks,
Joel Hestness
Joel Hestness
2015-07-15 20:00:31 UTC
Permalink
Post by Joel Hestness
src/mem/ruby/network/MessageBuffer.py, line 39
<http://reviews.gem5.org/r/2845/diff/4/?file=46645#file46645line39>
Why is a default recycle latency not set?
Why is this "Parent.recycle_latency"? Does both swtiches and controllers define a recycle latency? It doesn't appear so with this patch. I'm a bit confused how this actually works. I do not believe switches will ever use recycles, so it doesn't make much sense to define this default in the switch parent. Also many protocols avoid using recycles. Those protocols should not need to specify a recycle latency either.
Here again, I just copied the convention that was used when these were in SLICC. Specifically, AbstractControllers require a recycle_latency to be defined, and it was used to set their MessageBuffer recycle_latency (you can see this in the SLICC code I removed in this patch). MessageBuffers in the network never set a default (was probably a bad idea).
1) Define a default of the parent's recycle_latency parameter
2) Define a default of 0
If we go with (2), there wouldn't be a way to set a single recycle_latency for all of a controller's MessageBuffers. We'd need to explicitly set them all when non-zero. This sounds like it would result in tons of latent bugs.
I think (1) is best: For MessageBuffers within controllers, recycle_latency will at least be set with the controller parent's value, and the user can explictly override this value by setting any particular buffer's recycle_latency in the config files. Network MessageBuffers should explicitly set recycle_latency to indicate that there is no latency.
A possibility to address your desire would be to add, in SimpleNetwork and SimpleRouter, a recycle_latency parameter that defaults to 0. That way, the MessageBuffers would inherit these values and could still be overridden by a user. Would that be agreeable?
It sounds like we agree that recycling and setting a network recycle latency does not make much sense for network MessageBuffers. Since the network never sets a default recycle_latency, I'm still confused how the current code even works. It seems like the SimpleNetwork and SimpleRouter must set a recycle_latency parameter that defaults to 0.
Recycle latency is really a controller feature. I wonder if the best way to resolve this confusion is to remove the MessageBuffer recycle latency parameter and just have the controllers pass their recycle latency to the recycle() function.
Post by Brad Beckmann
Since the network never sets a default recycle_latency, I'm still confused how the current code even works. It seems like the SimpleNetwork and SimpleRouter must set a recycle_latency parameter that defaults to 0.
This patch explicitly specifies recycle_latency=0 for all network/switch MessageBuffers during their instantiation (this is precisely what you took issue with in your other review...). Neither SimpleNetwork or SimpleRouter have a recycle_latency parameter.
Post by Brad Beckmann
Recycle latency is really a controller feature. I wonder if the best way to resolve this confusion is to remove the MessageBuffer recycle latency parameter and just have the controllers pass their recycle latency to the recycle() function.
Maybe so, but this is outside the scope of this patch, which just aims to promote MessageBuffers to SimObjects. The MessageBuffer's recycle_latency variable is the same as it was previously, and this patch improves its setting by explicitly setting the value to 0 for network buffers (which was not happening previously). If we want to remove this variable from MessageBuffer, I think it should be done in a separate patch.
Network MessageBuffers did not set the value to 0 because it was never used. By making MessageBuffers SimObjects, you are changing how MessageBuffes are initiated. If you want to minimize the changes, then you should choose option (2) "define a default of 0" and make sure that the controllers overwrite the value. That is most similar to what is done before this patch. The parent recycle_latency trick is cool, but some parents (network/switches) don't have a recycle latency.
Since you want to do option (1) how about we compromize and say move loops to SimpleNetwork and SimpleLink.py and set the Network and Switch recycle latency to zero. In the Network and Switch add a comment that makes it clear that these parameters don't make much sense.
Ok. I've added a default recycle_latency parameter to SimpleNetwork and Switch.

I think I agree that the scope of recycle latencies should be confined to the SLICC controllers and passed to the recycle() function. I looked into what it would take to do this, and it's very non-trivial. SLICC controllers inherit from AbstractController, but SLICC does not expose AbstractController member variables to be used within controller definitions (*.sm files). Thus, in the current codebase, the m_recycle_latency variable of the AbstractController cannot be passed to recycle() in controllers that need to recycle MessageBuffers. I think we'd either need to (1) move recycle latency from being an AbstractController member variable to being parameters of the controllers that actually do recycles, or (2) change SLICC to provide visibility of AbstractController member variables inside of controller definitions.


- Joel


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2845/#review6548
-----------------------------------------------------------
Post by Joel Hestness
-----------------------------------------------------------
http://reviews.gem5.org/r/2845/
-----------------------------------------------------------
(Updated June 20, 2015, 4:14 p.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 10859:d0dddb8fc9de
---------------------------
ruby: Expose MessageBuffers as SimObjects
Expose MessageBuffers from SLICC controllers as SimObjects that can be
1) First and foremost, it exposes MessageBuffers as SimObjects that can be
manipulated in Python code. This allows parameters to be set and checked in
Python code to avoid obfuscating parameters within protocol files. Further, now
as SimObjects, MessageBuffer parameters are printed to config output files as a
way to track parameters across simulations (e.g. buffer sizes)
2) Cleans up special-case code for responseFromMemory buffers, and aligns their
instantiation and use with mandatoryQueue buffers. These two special buffers
are the only MessageBuffers that are exposed to components outside of SLICC
controllers, and they're both slave ends of these buffers. They should be
exposed outside of SLICC in the same way, and this patch does it.
3) Distinguishes buffer-specific parameters from buffer-to-network parameters.
Specifically, buffer size, randomization, ordering, recycle latency, and ports
are all specific to a MessageBuffer, while the virtual network ID and type are
intrinsics of how the buffer is connected to network ports. The former are
specified in the Python object, while the latter are specified in the
controller *.sm files. Unlike buffer-specific parameters, which may need to
change depending on the simulated system structure, buffer-to-network
parameters can be specified statically for most or all different simulated
systems.
Diffs
-----
configs/ruby/Ruby.py d02b45a554b5
src/mem/protocol/RubySlicc_Defines.sm d02b45a554b5
src/mem/ruby/SConscript d02b45a554b5
src/mem/ruby/network/MessageBuffer.hh d02b45a554b5
src/mem/ruby/network/MessageBuffer.cc d02b45a554b5
src/mem/ruby/network/MessageBuffer.py PRE-CREATION
src/mem/ruby/network/SConscript d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.hh d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.cc d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.py d02b45a554b5
src/mem/ruby/network/simple/Switch.hh d02b45a554b5
src/mem/ruby/network/simple/Switch.cc d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.hh d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.cc d02b45a554b5
src/mem/slicc/ast/ObjDeclAST.py d02b45a554b5
src/mem/slicc/symbols/StateMachine.py d02b45a554b5
src/python/swig/pyobject.cc d02b45a554b5
Diff: http://reviews.gem5.org/r/2845/diff/
Testing
-------
Regression tests with all Ruby protocols. Protocol file changes are in
separate patch (http://reviews.gem5.org/r/2844/) to keep this one slim.
Thanks,
Joel Hestness
Brad Beckmann
2015-07-21 23:51:30 UTC
Permalink
Post by Joel Hestness
src/mem/ruby/network/MessageBuffer.py, line 39
<http://reviews.gem5.org/r/2845/diff/4/?file=46645#file46645line39>
Why is a default recycle latency not set?
Why is this "Parent.recycle_latency"? Does both swtiches and controllers define a recycle latency? It doesn't appear so with this patch. I'm a bit confused how this actually works. I do not believe switches will ever use recycles, so it doesn't make much sense to define this default in the switch parent. Also many protocols avoid using recycles. Those protocols should not need to specify a recycle latency either.
Here again, I just copied the convention that was used when these were in SLICC. Specifically, AbstractControllers require a recycle_latency to be defined, and it was used to set their MessageBuffer recycle_latency (you can see this in the SLICC code I removed in this patch). MessageBuffers in the network never set a default (was probably a bad idea).
1) Define a default of the parent's recycle_latency parameter
2) Define a default of 0
If we go with (2), there wouldn't be a way to set a single recycle_latency for all of a controller's MessageBuffers. We'd need to explicitly set them all when non-zero. This sounds like it would result in tons of latent bugs.
I think (1) is best: For MessageBuffers within controllers, recycle_latency will at least be set with the controller parent's value, and the user can explictly override this value by setting any particular buffer's recycle_latency in the config files. Network MessageBuffers should explicitly set recycle_latency to indicate that there is no latency.
A possibility to address your desire would be to add, in SimpleNetwork and SimpleRouter, a recycle_latency parameter that defaults to 0. That way, the MessageBuffers would inherit these values and could still be overridden by a user. Would that be agreeable?
It sounds like we agree that recycling and setting a network recycle latency does not make much sense for network MessageBuffers. Since the network never sets a default recycle_latency, I'm still confused how the current code even works. It seems like the SimpleNetwork and SimpleRouter must set a recycle_latency parameter that defaults to 0.
Recycle latency is really a controller feature. I wonder if the best way to resolve this confusion is to remove the MessageBuffer recycle latency parameter and just have the controllers pass their recycle latency to the recycle() function.
Post by Brad Beckmann
Since the network never sets a default recycle_latency, I'm still confused how the current code even works. It seems like the SimpleNetwork and SimpleRouter must set a recycle_latency parameter that defaults to 0.
This patch explicitly specifies recycle_latency=0 for all network/switch MessageBuffers during their instantiation (this is precisely what you took issue with in your other review...). Neither SimpleNetwork or SimpleRouter have a recycle_latency parameter.
Post by Brad Beckmann
Recycle latency is really a controller feature. I wonder if the best way to resolve this confusion is to remove the MessageBuffer recycle latency parameter and just have the controllers pass their recycle latency to the recycle() function.
Maybe so, but this is outside the scope of this patch, which just aims to promote MessageBuffers to SimObjects. The MessageBuffer's recycle_latency variable is the same as it was previously, and this patch improves its setting by explicitly setting the value to 0 for network buffers (which was not happening previously). If we want to remove this variable from MessageBuffer, I think it should be done in a separate patch.
Network MessageBuffers did not set the value to 0 because it was never used. By making MessageBuffers SimObjects, you are changing how MessageBuffes are initiated. If you want to minimize the changes, then you should choose option (2) "define a default of 0" and make sure that the controllers overwrite the value. That is most similar to what is done before this patch. The parent recycle_latency trick is cool, but some parents (network/switches) don't have a recycle latency.
Since you want to do option (1) how about we compromize and say move loops to SimpleNetwork and SimpleLink.py and set the Network and Switch recycle latency to zero. In the Network and Switch add a comment that makes it clear that these parameters don't make much sense.
Ok. I've added a default recycle_latency parameter to SimpleNetwork and Switch.
I think I agree that the scope of recycle latencies should be confined to the SLICC controllers and passed to the recycle() function. I looked into what it would take to do this, and it's very non-trivial. SLICC controllers inherit from AbstractController, but SLICC does not expose AbstractController member variables to be used within controller definitions (*.sm files). Thus, in the current codebase, the m_recycle_latency variable of the AbstractController cannot be passed to recycle() in controllers that need to recycle MessageBuffers. I think we'd either need to (1) move recycle latency from being an AbstractController member variable to being parameters of the controllers that actually do recycles, or (2) change SLICC to provide visibility of AbstractController member variables inside of controller definitions.
Can you repost the patch with this change? As long as there's a comment explaining why the SimpleNetwork and Switch objects have recycle latencies, I think that will be fine.

Eventually, I hope we just get rid of recycles. They've caused a lot of pain over the past 12-13 years.


- Brad


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2845/#review6548
-----------------------------------------------------------
Post by Joel Hestness
-----------------------------------------------------------
http://reviews.gem5.org/r/2845/
-----------------------------------------------------------
(Updated June 20, 2015, 4:14 p.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 10859:d0dddb8fc9de
---------------------------
ruby: Expose MessageBuffers as SimObjects
Expose MessageBuffers from SLICC controllers as SimObjects that can be
1) First and foremost, it exposes MessageBuffers as SimObjects that can be
manipulated in Python code. This allows parameters to be set and checked in
Python code to avoid obfuscating parameters within protocol files. Further, now
as SimObjects, MessageBuffer parameters are printed to config output files as a
way to track parameters across simulations (e.g. buffer sizes)
2) Cleans up special-case code for responseFromMemory buffers, and aligns their
instantiation and use with mandatoryQueue buffers. These two special buffers
are the only MessageBuffers that are exposed to components outside of SLICC
controllers, and they're both slave ends of these buffers. They should be
exposed outside of SLICC in the same way, and this patch does it.
3) Distinguishes buffer-specific parameters from buffer-to-network parameters.
Specifically, buffer size, randomization, ordering, recycle latency, and ports
are all specific to a MessageBuffer, while the virtual network ID and type are
intrinsics of how the buffer is connected to network ports. The former are
specified in the Python object, while the latter are specified in the
controller *.sm files. Unlike buffer-specific parameters, which may need to
change depending on the simulated system structure, buffer-to-network
parameters can be specified statically for most or all different simulated
systems.
Diffs
-----
configs/ruby/Ruby.py d02b45a554b5
src/mem/protocol/RubySlicc_Defines.sm d02b45a554b5
src/mem/ruby/SConscript d02b45a554b5
src/mem/ruby/network/MessageBuffer.hh d02b45a554b5
src/mem/ruby/network/MessageBuffer.cc d02b45a554b5
src/mem/ruby/network/MessageBuffer.py PRE-CREATION
src/mem/ruby/network/SConscript d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.hh d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.cc d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.py d02b45a554b5
src/mem/ruby/network/simple/Switch.hh d02b45a554b5
src/mem/ruby/network/simple/Switch.cc d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.hh d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.cc d02b45a554b5
src/mem/slicc/ast/ObjDeclAST.py d02b45a554b5
src/mem/slicc/symbols/StateMachine.py d02b45a554b5
src/python/swig/pyobject.cc d02b45a554b5
Diff: http://reviews.gem5.org/r/2845/diff/
Testing
-------
Regression tests with all Ruby protocols. Protocol file changes are in
separate patch (http://reviews.gem5.org/r/2844/) to keep this one slim.
Thanks,
Joel Hestness
Joel Hestness
2015-06-24 16:54:42 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2845/#review6591
-----------------------------------------------------------



configs/ruby/Ruby.py (line 92)
<http://reviews.gem5.org/r/2845/#comment5689>

Splitting Brad's comments for easier tracking: "Overall, can this code be moved into the Switch and Liny .py files rather than have it here in Ruby.py?"



configs/ruby/Ruby.py (line 97)
<http://reviews.gem5.org/r/2845/#comment5691>

Splitting Brad's comments for easier tracking: "Finally, please correct me if I'm wrong, but I believe that all message buffers within the network have always been ordered, correct? If so, this patch maintains that same functionality, which is great. It would be good to add a comment pointing that out."



configs/ruby/Ruby.py (line 100)
<http://reviews.gem5.org/r/2845/#comment5690>

Splitting Brad's comments for easier tracking: "Also can we avoid setting the recycle_latency variable and just specify the default in the MessageBuffer.py file?"


- Joel Hestness
Post by Joel Hestness
-----------------------------------------------------------
http://reviews.gem5.org/r/2845/
-----------------------------------------------------------
(Updated June 20, 2015, 4:14 p.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 10859:d0dddb8fc9de
---------------------------
ruby: Expose MessageBuffers as SimObjects
Expose MessageBuffers from SLICC controllers as SimObjects that can be
1) First and foremost, it exposes MessageBuffers as SimObjects that can be
manipulated in Python code. This allows parameters to be set and checked in
Python code to avoid obfuscating parameters within protocol files. Further, now
as SimObjects, MessageBuffer parameters are printed to config output files as a
way to track parameters across simulations (e.g. buffer sizes)
2) Cleans up special-case code for responseFromMemory buffers, and aligns their
instantiation and use with mandatoryQueue buffers. These two special buffers
are the only MessageBuffers that are exposed to components outside of SLICC
controllers, and they're both slave ends of these buffers. They should be
exposed outside of SLICC in the same way, and this patch does it.
3) Distinguishes buffer-specific parameters from buffer-to-network parameters.
Specifically, buffer size, randomization, ordering, recycle latency, and ports
are all specific to a MessageBuffer, while the virtual network ID and type are
intrinsics of how the buffer is connected to network ports. The former are
specified in the Python object, while the latter are specified in the
controller *.sm files. Unlike buffer-specific parameters, which may need to
change depending on the simulated system structure, buffer-to-network
parameters can be specified statically for most or all different simulated
systems.
Diffs
-----
configs/ruby/Ruby.py d02b45a554b5
src/mem/protocol/RubySlicc_Defines.sm d02b45a554b5
src/mem/ruby/SConscript d02b45a554b5
src/mem/ruby/network/MessageBuffer.hh d02b45a554b5
src/mem/ruby/network/MessageBuffer.cc d02b45a554b5
src/mem/ruby/network/MessageBuffer.py PRE-CREATION
src/mem/ruby/network/SConscript d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.hh d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.cc d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.py d02b45a554b5
src/mem/ruby/network/simple/Switch.hh d02b45a554b5
src/mem/ruby/network/simple/Switch.cc d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.hh d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.cc d02b45a554b5
src/mem/slicc/ast/ObjDeclAST.py d02b45a554b5
src/mem/slicc/symbols/StateMachine.py d02b45a554b5
src/python/swig/pyobject.cc d02b45a554b5
Diff: http://reviews.gem5.org/r/2845/diff/
Testing
-------
Regression tests with all Ruby protocols. Protocol file changes are in
separate patch (http://reviews.gem5.org/r/2844/) to keep this one slim.
Thanks,
Joel Hestness
Joel Hestness
2015-06-24 17:30:58 UTC
Permalink
Post by Joel Hestness
configs/ruby/Ruby.py, line 92
<http://reviews.gem5.org/r/2845/diff/4/?file=46640#file46640line92>
Splitting Brad's comments for easier tracking: "Overall, can this code be moved into the Switch and Liny .py files rather than have it here in Ruby.py?"
Actually, I don't think it make sense to move these into the links, switches or the SimpleNetwork. By setting up buffers here, the user can modify buffer parameters in the configs and not require recompilation. If we move this setup into, for example, SimpleNetwork.py - which knows how buffers need to be instantiated and connected, then a user would need find the buffers they want to modify in the config files here, after the call to network.setup_buffers(), and then override their parameters. OR they would have to modify the parameters in one of the SimObject declarations and then recompile the simulator. At least with the setup as-is, it is easy to identify and modify buffer parameters.

Would it be ok to leave this?


- Joel


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2845/#review6591
-----------------------------------------------------------
Post by Joel Hestness
-----------------------------------------------------------
http://reviews.gem5.org/r/2845/
-----------------------------------------------------------
(Updated June 20, 2015, 4:14 p.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 10859:d0dddb8fc9de
---------------------------
ruby: Expose MessageBuffers as SimObjects
Expose MessageBuffers from SLICC controllers as SimObjects that can be
1) First and foremost, it exposes MessageBuffers as SimObjects that can be
manipulated in Python code. This allows parameters to be set and checked in
Python code to avoid obfuscating parameters within protocol files. Further, now
as SimObjects, MessageBuffer parameters are printed to config output files as a
way to track parameters across simulations (e.g. buffer sizes)
2) Cleans up special-case code for responseFromMemory buffers, and aligns their
instantiation and use with mandatoryQueue buffers. These two special buffers
are the only MessageBuffers that are exposed to components outside of SLICC
controllers, and they're both slave ends of these buffers. They should be
exposed outside of SLICC in the same way, and this patch does it.
3) Distinguishes buffer-specific parameters from buffer-to-network parameters.
Specifically, buffer size, randomization, ordering, recycle latency, and ports
are all specific to a MessageBuffer, while the virtual network ID and type are
intrinsics of how the buffer is connected to network ports. The former are
specified in the Python object, while the latter are specified in the
controller *.sm files. Unlike buffer-specific parameters, which may need to
change depending on the simulated system structure, buffer-to-network
parameters can be specified statically for most or all different simulated
systems.
Diffs
-----
configs/ruby/Ruby.py d02b45a554b5
src/mem/protocol/RubySlicc_Defines.sm d02b45a554b5
src/mem/ruby/SConscript d02b45a554b5
src/mem/ruby/network/MessageBuffer.hh d02b45a554b5
src/mem/ruby/network/MessageBuffer.cc d02b45a554b5
src/mem/ruby/network/MessageBuffer.py PRE-CREATION
src/mem/ruby/network/SConscript d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.hh d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.cc d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.py d02b45a554b5
src/mem/ruby/network/simple/Switch.hh d02b45a554b5
src/mem/ruby/network/simple/Switch.cc d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.hh d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.cc d02b45a554b5
src/mem/slicc/ast/ObjDeclAST.py d02b45a554b5
src/mem/slicc/symbols/StateMachine.py d02b45a554b5
src/python/swig/pyobject.cc d02b45a554b5
Diff: http://reviews.gem5.org/r/2845/diff/
Testing
-------
Regression tests with all Ruby protocols. Protocol file changes are in
separate patch (http://reviews.gem5.org/r/2844/) to keep this one slim.
Thanks,
Joel Hestness
Brad Beckmann
2015-06-26 15:48:26 UTC
Permalink
Post by Joel Hestness
configs/ruby/Ruby.py, line 92
<http://reviews.gem5.org/r/2845/diff/4/?file=46640#file46640line92>
Splitting Brad's comments for easier tracking: "Overall, can this code be moved into the Switch and Liny .py files rather than have it here in Ruby.py?"
Actually, I don't think it make sense to move these into the links, switches or the SimpleNetwork. By setting up buffers here, the user can modify buffer parameters in the configs and not require recompilation. If we move this setup into, for example, SimpleNetwork.py - which knows how buffers need to be instantiated and connected, then a user would need find the buffers they want to modify in the config files here, after the call to network.setup_buffers(), and then override their parameters. OR they would have to modify the parameters in one of the SimObject declarations and then recompile the simulator. At least with the setup as-is, it is easy to identify and modify buffer parameters.
Would it be ok to leave this?
On can still modify the buffer parameters even if you move this code into the Switch and Link .py files. All this code is doing is expanding each link into it's virtual channels. To modify the MessageBuffers parameters, the user should be modifying the topology file. That is where they know which switch/link is which.

Also let's rename the network.message_buffers network.int_link_buffers or network.interface_buffers. That way folks know the difference between the message buffers assoicated with the network and those associated with the switches.


- Brad


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2845/#review6591
-----------------------------------------------------------
Post by Joel Hestness
-----------------------------------------------------------
http://reviews.gem5.org/r/2845/
-----------------------------------------------------------
(Updated June 20, 2015, 4:14 p.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 10859:d0dddb8fc9de
---------------------------
ruby: Expose MessageBuffers as SimObjects
Expose MessageBuffers from SLICC controllers as SimObjects that can be
1) First and foremost, it exposes MessageBuffers as SimObjects that can be
manipulated in Python code. This allows parameters to be set and checked in
Python code to avoid obfuscating parameters within protocol files. Further, now
as SimObjects, MessageBuffer parameters are printed to config output files as a
way to track parameters across simulations (e.g. buffer sizes)
2) Cleans up special-case code for responseFromMemory buffers, and aligns their
instantiation and use with mandatoryQueue buffers. These two special buffers
are the only MessageBuffers that are exposed to components outside of SLICC
controllers, and they're both slave ends of these buffers. They should be
exposed outside of SLICC in the same way, and this patch does it.
3) Distinguishes buffer-specific parameters from buffer-to-network parameters.
Specifically, buffer size, randomization, ordering, recycle latency, and ports
are all specific to a MessageBuffer, while the virtual network ID and type are
intrinsics of how the buffer is connected to network ports. The former are
specified in the Python object, while the latter are specified in the
controller *.sm files. Unlike buffer-specific parameters, which may need to
change depending on the simulated system structure, buffer-to-network
parameters can be specified statically for most or all different simulated
systems.
Diffs
-----
configs/ruby/Ruby.py d02b45a554b5
src/mem/protocol/RubySlicc_Defines.sm d02b45a554b5
src/mem/ruby/SConscript d02b45a554b5
src/mem/ruby/network/MessageBuffer.hh d02b45a554b5
src/mem/ruby/network/MessageBuffer.cc d02b45a554b5
src/mem/ruby/network/MessageBuffer.py PRE-CREATION
src/mem/ruby/network/SConscript d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.hh d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.cc d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.py d02b45a554b5
src/mem/ruby/network/simple/Switch.hh d02b45a554b5
src/mem/ruby/network/simple/Switch.cc d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.hh d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.cc d02b45a554b5
src/mem/slicc/ast/ObjDeclAST.py d02b45a554b5
src/mem/slicc/symbols/StateMachine.py d02b45a554b5
src/python/swig/pyobject.cc d02b45a554b5
Diff: http://reviews.gem5.org/r/2845/diff/
Testing
-------
Regression tests with all Ruby protocols. Protocol file changes are in
separate patch (http://reviews.gem5.org/r/2844/) to keep this one slim.
Thanks,
Joel Hestness
Joel Hestness
2015-06-26 16:33:11 UTC
Permalink
Post by Joel Hestness
configs/ruby/Ruby.py, line 92
<http://reviews.gem5.org/r/2845/diff/4/?file=46640#file46640line92>
Splitting Brad's comments for easier tracking: "Overall, can this code be moved into the Switch and Liny .py files rather than have it here in Ruby.py?"
Actually, I don't think it make sense to move these into the links, switches or the SimpleNetwork. By setting up buffers here, the user can modify buffer parameters in the configs and not require recompilation. If we move this setup into, for example, SimpleNetwork.py - which knows how buffers need to be instantiated and connected, then a user would need find the buffers they want to modify in the config files here, after the call to network.setup_buffers(), and then override their parameters. OR they would have to modify the parameters in one of the SimObject declarations and then recompile the simulator. At least with the setup as-is, it is easy to identify and modify buffer parameters.
Would it be ok to leave this?
On can still modify the buffer parameters even if you move this code into the Switch and Link .py files. All this code is doing is expanding each link into it's virtual channels. To modify the MessageBuffers parameters, the user should be modifying the topology file. That is where they know which switch/link is which.
Also let's rename the network.message_buffers network.int_link_buffers or network.interface_buffers. That way folks know the difference between the message buffers assoicated with the network and those associated with the switches.
On can still modify the buffer parameters even if you move this code into the Switch and Link .py files. All this code is doing is expanding each link into it's virtual channels. To modify the MessageBuffers parameters, the user should be modifying the topology file. That is where they know which switch/link is which.
I apologize, but I'm must be missing what you mean by "moving this code into Switch and Link .py files". I can sort of see how links could parent their MessageBuffers, since the network just passes its buffers through to the int_links. However, I think that change is outside the scope of this patch, since the aim of this patch is promoting MessageBuffers to SimObjects rather than changing how links use MessageBuffers.

Moving buffers into Switch.py is confusing because not every switch port has buffers for VCs. Only ports between switches, and int_links/out_links require buffers. Thus, the topology matters in deciding where to instantiate MessageBuffers. However, the topology does NOT define the VC architecture. It only defines the connections between network components. The SimpleNetwork is the only component that knows about switch connections and VC architecture.

Can you clarify what you mean here?
Post by Joel Hestness
Also let's rename the network.message_buffers network.int_link_buffers or network.interface_buffers. That way folks know the difference between the message buffers assoicated with the network and those associated with the switches.
This also confuses me. Buffers associated with the network links are children of the network, while buffers associated with switches are children of the switches. I can change the buffer vector's name to network.int_link_buffers, but that won't clarify anything the way you suggest. Was there something more you were hoping for here?


- Joel


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2845/#review6591
-----------------------------------------------------------
Post by Joel Hestness
-----------------------------------------------------------
http://reviews.gem5.org/r/2845/
-----------------------------------------------------------
(Updated June 20, 2015, 4:14 p.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 10859:d0dddb8fc9de
---------------------------
ruby: Expose MessageBuffers as SimObjects
Expose MessageBuffers from SLICC controllers as SimObjects that can be
1) First and foremost, it exposes MessageBuffers as SimObjects that can be
manipulated in Python code. This allows parameters to be set and checked in
Python code to avoid obfuscating parameters within protocol files. Further, now
as SimObjects, MessageBuffer parameters are printed to config output files as a
way to track parameters across simulations (e.g. buffer sizes)
2) Cleans up special-case code for responseFromMemory buffers, and aligns their
instantiation and use with mandatoryQueue buffers. These two special buffers
are the only MessageBuffers that are exposed to components outside of SLICC
controllers, and they're both slave ends of these buffers. They should be
exposed outside of SLICC in the same way, and this patch does it.
3) Distinguishes buffer-specific parameters from buffer-to-network parameters.
Specifically, buffer size, randomization, ordering, recycle latency, and ports
are all specific to a MessageBuffer, while the virtual network ID and type are
intrinsics of how the buffer is connected to network ports. The former are
specified in the Python object, while the latter are specified in the
controller *.sm files. Unlike buffer-specific parameters, which may need to
change depending on the simulated system structure, buffer-to-network
parameters can be specified statically for most or all different simulated
systems.
Diffs
-----
configs/ruby/Ruby.py d02b45a554b5
src/mem/protocol/RubySlicc_Defines.sm d02b45a554b5
src/mem/ruby/SConscript d02b45a554b5
src/mem/ruby/network/MessageBuffer.hh d02b45a554b5
src/mem/ruby/network/MessageBuffer.cc d02b45a554b5
src/mem/ruby/network/MessageBuffer.py PRE-CREATION
src/mem/ruby/network/SConscript d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.hh d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.cc d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.py d02b45a554b5
src/mem/ruby/network/simple/Switch.hh d02b45a554b5
src/mem/ruby/network/simple/Switch.cc d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.hh d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.cc d02b45a554b5
src/mem/slicc/ast/ObjDeclAST.py d02b45a554b5
src/mem/slicc/symbols/StateMachine.py d02b45a554b5
src/python/swig/pyobject.cc d02b45a554b5
Diff: http://reviews.gem5.org/r/2845/diff/
Testing
-------
Regression tests with all Ruby protocols. Protocol file changes are in
separate patch (http://reviews.gem5.org/r/2844/) to keep this one slim.
Thanks,
Joel Hestness
Brad Beckmann
2015-06-26 16:57:22 UTC
Permalink
Post by Joel Hestness
configs/ruby/Ruby.py, line 92
<http://reviews.gem5.org/r/2845/diff/4/?file=46640#file46640line92>
Splitting Brad's comments for easier tracking: "Overall, can this code be moved into the Switch and Liny .py files rather than have it here in Ruby.py?"
Actually, I don't think it make sense to move these into the links, switches or the SimpleNetwork. By setting up buffers here, the user can modify buffer parameters in the configs and not require recompilation. If we move this setup into, for example, SimpleNetwork.py - which knows how buffers need to be instantiated and connected, then a user would need find the buffers they want to modify in the config files here, after the call to network.setup_buffers(), and then override their parameters. OR they would have to modify the parameters in one of the SimObject declarations and then recompile the simulator. At least with the setup as-is, it is easy to identify and modify buffer parameters.
Would it be ok to leave this?
On can still modify the buffer parameters even if you move this code into the Switch and Link .py files. All this code is doing is expanding each link into it's virtual channels. To modify the MessageBuffers parameters, the user should be modifying the topology file. That is where they know which switch/link is which.
Also let's rename the network.message_buffers network.int_link_buffers or network.interface_buffers. That way folks know the difference between the message buffers assoicated with the network and those associated with the switches.
Post by Brad Beckmann
On can still modify the buffer parameters even if you move this code into the Switch and Link .py files. All this code is doing is expanding each link into it's virtual channels. To modify the MessageBuffers parameters, the user should be modifying the topology file. That is where they know which switch/link is which.
I apologize, but I'm must be missing what you mean by "moving this code into Switch and Link .py files". I can sort of see how links could parent their MessageBuffers, since the network just passes its buffers through to the int_links. However, I think that change is outside the scope of this patch, since the aim of this patch is promoting MessageBuffers to SimObjects rather than changing how links use MessageBuffers.
Moving buffers into Switch.py is confusing because not every switch port has buffers for VCs. Only ports between switches, and int_links/out_links require buffers. Thus, the topology matters in deciding where to instantiate MessageBuffers. However, the topology does NOT define the VC architecture. It only defines the connections between network components. The SimpleNetwork is the only component that knows about switch connections and VC architecture.
Can you clarify what you mean here?
Post by Brad Beckmann
Also let's rename the network.message_buffers network.int_link_buffers or network.interface_buffers. That way folks know the difference between the message buffers assoicated with the network and those associated with the switches.
This also confuses me. Buffers associated with the network links are children of the network, while buffers associated with switches are children of the switches. I can change the buffer vector's name to network.int_link_buffers, but that won't clarify anything the way you suggest. Was there something more you were hoping for here?
All I'm asking is to move the first and third loops from Ruby.py to SimpleLink.py and the second loop from Ruby.py to SimpleNetwork.py. I'm not asking for a big change here. I'm certainly not asking to change how links use MessageBuffers.

When you say VC architecture, are you referring to something other than the number of VCs? The number of VCs is a parameter from the RubyNetwork and is easily accessible to the SimpleLink and SimpleNetwork (inherits from RubyNetwork).

The VC design is part of the protocol. Often Topology files are protocol specific, so if you want to do something fancy with VC buffers, it seems like the Topology file is were to do it.

What is a "switch port" and which switch port does not have MessageBuffers for VCs?


- Brad


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2845/#review6591
-----------------------------------------------------------
Post by Joel Hestness
-----------------------------------------------------------
http://reviews.gem5.org/r/2845/
-----------------------------------------------------------
(Updated June 20, 2015, 4:14 p.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 10859:d0dddb8fc9de
---------------------------
ruby: Expose MessageBuffers as SimObjects
Expose MessageBuffers from SLICC controllers as SimObjects that can be
1) First and foremost, it exposes MessageBuffers as SimObjects that can be
manipulated in Python code. This allows parameters to be set and checked in
Python code to avoid obfuscating parameters within protocol files. Further, now
as SimObjects, MessageBuffer parameters are printed to config output files as a
way to track parameters across simulations (e.g. buffer sizes)
2) Cleans up special-case code for responseFromMemory buffers, and aligns their
instantiation and use with mandatoryQueue buffers. These two special buffers
are the only MessageBuffers that are exposed to components outside of SLICC
controllers, and they're both slave ends of these buffers. They should be
exposed outside of SLICC in the same way, and this patch does it.
3) Distinguishes buffer-specific parameters from buffer-to-network parameters.
Specifically, buffer size, randomization, ordering, recycle latency, and ports
are all specific to a MessageBuffer, while the virtual network ID and type are
intrinsics of how the buffer is connected to network ports. The former are
specified in the Python object, while the latter are specified in the
controller *.sm files. Unlike buffer-specific parameters, which may need to
change depending on the simulated system structure, buffer-to-network
parameters can be specified statically for most or all different simulated
systems.
Diffs
-----
configs/ruby/Ruby.py d02b45a554b5
src/mem/protocol/RubySlicc_Defines.sm d02b45a554b5
src/mem/ruby/SConscript d02b45a554b5
src/mem/ruby/network/MessageBuffer.hh d02b45a554b5
src/mem/ruby/network/MessageBuffer.cc d02b45a554b5
src/mem/ruby/network/MessageBuffer.py PRE-CREATION
src/mem/ruby/network/SConscript d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.hh d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.cc d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.py d02b45a554b5
src/mem/ruby/network/simple/Switch.hh d02b45a554b5
src/mem/ruby/network/simple/Switch.cc d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.hh d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.cc d02b45a554b5
src/mem/slicc/ast/ObjDeclAST.py d02b45a554b5
src/mem/slicc/symbols/StateMachine.py d02b45a554b5
src/python/swig/pyobject.cc d02b45a554b5
Diff: http://reviews.gem5.org/r/2845/diff/
Testing
-------
Regression tests with all Ruby protocols. Protocol file changes are in
separate patch (http://reviews.gem5.org/r/2844/) to keep this one slim.
Thanks,
Joel Hestness
Joel Hestness
2015-07-15 20:06:05 UTC
Permalink
Post by Joel Hestness
configs/ruby/Ruby.py, line 92
<http://reviews.gem5.org/r/2845/diff/4/?file=46640#file46640line92>
Splitting Brad's comments for easier tracking: "Overall, can this code be moved into the Switch and Liny .py files rather than have it here in Ruby.py?"
Actually, I don't think it make sense to move these into the links, switches or the SimpleNetwork. By setting up buffers here, the user can modify buffer parameters in the configs and not require recompilation. If we move this setup into, for example, SimpleNetwork.py - which knows how buffers need to be instantiated and connected, then a user would need find the buffers they want to modify in the config files here, after the call to network.setup_buffers(), and then override their parameters. OR they would have to modify the parameters in one of the SimObject declarations and then recompile the simulator. At least with the setup as-is, it is easy to identify and modify buffer parameters.
Would it be ok to leave this?
On can still modify the buffer parameters even if you move this code into the Switch and Link .py files. All this code is doing is expanding each link into it's virtual channels. To modify the MessageBuffers parameters, the user should be modifying the topology file. That is where they know which switch/link is which.
Also let's rename the network.message_buffers network.int_link_buffers or network.interface_buffers. That way folks know the difference between the message buffers assoicated with the network and those associated with the switches.
Post by Brad Beckmann
On can still modify the buffer parameters even if you move this code into the Switch and Link .py files. All this code is doing is expanding each link into it's virtual channels. To modify the MessageBuffers parameters, the user should be modifying the topology file. That is where they know which switch/link is which.
I apologize, but I'm must be missing what you mean by "moving this code into Switch and Link .py files". I can sort of see how links could parent their MessageBuffers, since the network just passes its buffers through to the int_links. However, I think that change is outside the scope of this patch, since the aim of this patch is promoting MessageBuffers to SimObjects rather than changing how links use MessageBuffers.
Moving buffers into Switch.py is confusing because not every switch port has buffers for VCs. Only ports between switches, and int_links/out_links require buffers. Thus, the topology matters in deciding where to instantiate MessageBuffers. However, the topology does NOT define the VC architecture. It only defines the connections between network components. The SimpleNetwork is the only component that knows about switch connections and VC architecture.
Can you clarify what you mean here?
Post by Brad Beckmann
Also let's rename the network.message_buffers network.int_link_buffers or network.interface_buffers. That way folks know the difference between the message buffers assoicated with the network and those associated with the switches.
This also confuses me. Buffers associated with the network links are children of the network, while buffers associated with switches are children of the switches. I can change the buffer vector's name to network.int_link_buffers, but that won't clarify anything the way you suggest. Was there something more you were hoping for here?
All I'm asking is to move the first and third loops from Ruby.py to SimpleLink.py and the second loop from Ruby.py to SimpleNetwork.py. I'm not asking for a big change here. I'm certainly not asking to change how links use MessageBuffers.
When you say VC architecture, are you referring to something other than the number of VCs? The number of VCs is a parameter from the RubyNetwork and is easily accessible to the SimpleLink and SimpleNetwork (inherits from RubyNetwork).
The VC design is part of the protocol. Often Topology files are protocol specific, so if you want to do something fancy with VC buffers, it seems like the Topology file is were to do it.
What is a "switch port" and which switch port does not have MessageBuffers for VCs?
Sorry. I'm still confused about what you're requesting here.

In the setup_buffers function, there are 8 total loops, 4 of which are leaf loops that expand ports to VCs. Are you requesting that I move these port-to-VC expanding loops into other .py files? If so, the trouble is that Python VectorParams can only be assigned once, so we'd still need to maintain the network_buffers and router_buffers lists as local variables somewhere and accumulate the buffer instances before assigning them to network.int_link_buffers and router.buffers.

If you're requesting something else, I'd need some more direction. I can't figure out which loops (and code) you're referring to that be should moved.


- Joel


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2845/#review6591
-----------------------------------------------------------
Post by Joel Hestness
-----------------------------------------------------------
http://reviews.gem5.org/r/2845/
-----------------------------------------------------------
(Updated June 20, 2015, 4:14 p.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 10859:d0dddb8fc9de
---------------------------
ruby: Expose MessageBuffers as SimObjects
Expose MessageBuffers from SLICC controllers as SimObjects that can be
1) First and foremost, it exposes MessageBuffers as SimObjects that can be
manipulated in Python code. This allows parameters to be set and checked in
Python code to avoid obfuscating parameters within protocol files. Further, now
as SimObjects, MessageBuffer parameters are printed to config output files as a
way to track parameters across simulations (e.g. buffer sizes)
2) Cleans up special-case code for responseFromMemory buffers, and aligns their
instantiation and use with mandatoryQueue buffers. These two special buffers
are the only MessageBuffers that are exposed to components outside of SLICC
controllers, and they're both slave ends of these buffers. They should be
exposed outside of SLICC in the same way, and this patch does it.
3) Distinguishes buffer-specific parameters from buffer-to-network parameters.
Specifically, buffer size, randomization, ordering, recycle latency, and ports
are all specific to a MessageBuffer, while the virtual network ID and type are
intrinsics of how the buffer is connected to network ports. The former are
specified in the Python object, while the latter are specified in the
controller *.sm files. Unlike buffer-specific parameters, which may need to
change depending on the simulated system structure, buffer-to-network
parameters can be specified statically for most or all different simulated
systems.
Diffs
-----
configs/ruby/Ruby.py d02b45a554b5
src/mem/protocol/RubySlicc_Defines.sm d02b45a554b5
src/mem/ruby/SConscript d02b45a554b5
src/mem/ruby/network/MessageBuffer.hh d02b45a554b5
src/mem/ruby/network/MessageBuffer.cc d02b45a554b5
src/mem/ruby/network/MessageBuffer.py PRE-CREATION
src/mem/ruby/network/SConscript d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.hh d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.cc d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.py d02b45a554b5
src/mem/ruby/network/simple/Switch.hh d02b45a554b5
src/mem/ruby/network/simple/Switch.cc d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.hh d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.cc d02b45a554b5
src/mem/slicc/ast/ObjDeclAST.py d02b45a554b5
src/mem/slicc/symbols/StateMachine.py d02b45a554b5
src/python/swig/pyobject.cc d02b45a554b5
Diff: http://reviews.gem5.org/r/2845/diff/
Testing
-------
Regression tests with all Ruby protocols. Protocol file changes are in
separate patch (http://reviews.gem5.org/r/2844/) to keep this one slim.
Thanks,
Joel Hestness
Brad Beckmann
2015-07-21 23:51:33 UTC
Permalink
Post by Joel Hestness
configs/ruby/Ruby.py, line 92
<http://reviews.gem5.org/r/2845/diff/4/?file=46640#file46640line92>
Splitting Brad's comments for easier tracking: "Overall, can this code be moved into the Switch and Liny .py files rather than have it here in Ruby.py?"
Actually, I don't think it make sense to move these into the links, switches or the SimpleNetwork. By setting up buffers here, the user can modify buffer parameters in the configs and not require recompilation. If we move this setup into, for example, SimpleNetwork.py - which knows how buffers need to be instantiated and connected, then a user would need find the buffers they want to modify in the config files here, after the call to network.setup_buffers(), and then override their parameters. OR they would have to modify the parameters in one of the SimObject declarations and then recompile the simulator. At least with the setup as-is, it is easy to identify and modify buffer parameters.
Would it be ok to leave this?
On can still modify the buffer parameters even if you move this code into the Switch and Link .py files. All this code is doing is expanding each link into it's virtual channels. To modify the MessageBuffers parameters, the user should be modifying the topology file. That is where they know which switch/link is which.
Also let's rename the network.message_buffers network.int_link_buffers or network.interface_buffers. That way folks know the difference between the message buffers assoicated with the network and those associated with the switches.
Post by Brad Beckmann
On can still modify the buffer parameters even if you move this code into the Switch and Link .py files. All this code is doing is expanding each link into it's virtual channels. To modify the MessageBuffers parameters, the user should be modifying the topology file. That is where they know which switch/link is which.
I apologize, but I'm must be missing what you mean by "moving this code into Switch and Link .py files". I can sort of see how links could parent their MessageBuffers, since the network just passes its buffers through to the int_links. However, I think that change is outside the scope of this patch, since the aim of this patch is promoting MessageBuffers to SimObjects rather than changing how links use MessageBuffers.
Moving buffers into Switch.py is confusing because not every switch port has buffers for VCs. Only ports between switches, and int_links/out_links require buffers. Thus, the topology matters in deciding where to instantiate MessageBuffers. However, the topology does NOT define the VC architecture. It only defines the connections between network components. The SimpleNetwork is the only component that knows about switch connections and VC architecture.
Can you clarify what you mean here?
Post by Brad Beckmann
Also let's rename the network.message_buffers network.int_link_buffers or network.interface_buffers. That way folks know the difference between the message buffers assoicated with the network and those associated with the switches.
This also confuses me. Buffers associated with the network links are children of the network, while buffers associated with switches are children of the switches. I can change the buffer vector's name to network.int_link_buffers, but that won't clarify anything the way you suggest. Was there something more you were hoping for here?
All I'm asking is to move the first and third loops from Ruby.py to SimpleLink.py and the second loop from Ruby.py to SimpleNetwork.py. I'm not asking for a big change here. I'm certainly not asking to change how links use MessageBuffers.
When you say VC architecture, are you referring to something other than the number of VCs? The number of VCs is a parameter from the RubyNetwork and is easily accessible to the SimpleLink and SimpleNetwork (inherits from RubyNetwork).
The VC design is part of the protocol. Often Topology files are protocol specific, so if you want to do something fancy with VC buffers, it seems like the Topology file is were to do it.
What is a "switch port" and which switch port does not have MessageBuffers for VCs?
Sorry. I'm still confused about what you're requesting here.
In the setup_buffers function, there are 8 total loops, 4 of which are leaf loops that expand ports to VCs. Are you requesting that I move these port-to-VC expanding loops into other .py files? If so, the trouble is that Python VectorParams can only be assigned once, so we'd still need to maintain the network_buffers and router_buffers lists as local variables somewhere and accumulate the buffer instances before assigning them to network.int_link_buffers and router.buffers.
If you're requesting something else, I'd need some more direction. I can't figure out which loops (and code) you're referring to that be should moved.
Ok, I'll simplify my request. Please move the entire setup_buffers function to Network.py. My main goal here is not to add another function to Ruby.py unless it is absolutely necessary.

As an example, see the attachIO function in src/dev/x86/Pc.py. That is essentially what I'm looking for. The both functions are rather straight-forward and are simply connecting things together that are specific to a particular class.

Does that make sense?


- Brad


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2845/#review6591
-----------------------------------------------------------
Post by Joel Hestness
-----------------------------------------------------------
http://reviews.gem5.org/r/2845/
-----------------------------------------------------------
(Updated June 20, 2015, 4:14 p.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 10859:d0dddb8fc9de
---------------------------
ruby: Expose MessageBuffers as SimObjects
Expose MessageBuffers from SLICC controllers as SimObjects that can be
1) First and foremost, it exposes MessageBuffers as SimObjects that can be
manipulated in Python code. This allows parameters to be set and checked in
Python code to avoid obfuscating parameters within protocol files. Further, now
as SimObjects, MessageBuffer parameters are printed to config output files as a
way to track parameters across simulations (e.g. buffer sizes)
2) Cleans up special-case code for responseFromMemory buffers, and aligns their
instantiation and use with mandatoryQueue buffers. These two special buffers
are the only MessageBuffers that are exposed to components outside of SLICC
controllers, and they're both slave ends of these buffers. They should be
exposed outside of SLICC in the same way, and this patch does it.
3) Distinguishes buffer-specific parameters from buffer-to-network parameters.
Specifically, buffer size, randomization, ordering, recycle latency, and ports
are all specific to a MessageBuffer, while the virtual network ID and type are
intrinsics of how the buffer is connected to network ports. The former are
specified in the Python object, while the latter are specified in the
controller *.sm files. Unlike buffer-specific parameters, which may need to
change depending on the simulated system structure, buffer-to-network
parameters can be specified statically for most or all different simulated
systems.
Diffs
-----
configs/ruby/Ruby.py d02b45a554b5
src/mem/protocol/RubySlicc_Defines.sm d02b45a554b5
src/mem/ruby/SConscript d02b45a554b5
src/mem/ruby/network/MessageBuffer.hh d02b45a554b5
src/mem/ruby/network/MessageBuffer.cc d02b45a554b5
src/mem/ruby/network/MessageBuffer.py PRE-CREATION
src/mem/ruby/network/SConscript d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.hh d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.cc d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.py d02b45a554b5
src/mem/ruby/network/simple/Switch.hh d02b45a554b5
src/mem/ruby/network/simple/Switch.cc d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.hh d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.cc d02b45a554b5
src/mem/slicc/ast/ObjDeclAST.py d02b45a554b5
src/mem/slicc/symbols/StateMachine.py d02b45a554b5
src/python/swig/pyobject.cc d02b45a554b5
Diff: http://reviews.gem5.org/r/2845/diff/
Testing
-------
Regression tests with all Ruby protocols. Protocol file changes are in
separate patch (http://reviews.gem5.org/r/2844/) to keep this one slim.
Thanks,
Joel Hestness
Brad Beckmann
2015-06-26 16:57:19 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2845/#review6615
-----------------------------------------------------------



src/mem/ruby/network/simple/SimpleNetwork.py (line 42)
<http://reviews.gem5.org/r/2845/#comment5710>

Just change this name from buffers to int_link_buffers or interface_buffers.

I just want to make it clear this is not a list of all buffers in the network. Rather it is just those buffers not part of a Switch.


- Brad Beckmann
Post by Joel Hestness
-----------------------------------------------------------
http://reviews.gem5.org/r/2845/
-----------------------------------------------------------
(Updated June 20, 2015, 4:14 p.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 10859:d0dddb8fc9de
---------------------------
ruby: Expose MessageBuffers as SimObjects
Expose MessageBuffers from SLICC controllers as SimObjects that can be
1) First and foremost, it exposes MessageBuffers as SimObjects that can be
manipulated in Python code. This allows parameters to be set and checked in
Python code to avoid obfuscating parameters within protocol files. Further, now
as SimObjects, MessageBuffer parameters are printed to config output files as a
way to track parameters across simulations (e.g. buffer sizes)
2) Cleans up special-case code for responseFromMemory buffers, and aligns their
instantiation and use with mandatoryQueue buffers. These two special buffers
are the only MessageBuffers that are exposed to components outside of SLICC
controllers, and they're both slave ends of these buffers. They should be
exposed outside of SLICC in the same way, and this patch does it.
3) Distinguishes buffer-specific parameters from buffer-to-network parameters.
Specifically, buffer size, randomization, ordering, recycle latency, and ports
are all specific to a MessageBuffer, while the virtual network ID and type are
intrinsics of how the buffer is connected to network ports. The former are
specified in the Python object, while the latter are specified in the
controller *.sm files. Unlike buffer-specific parameters, which may need to
change depending on the simulated system structure, buffer-to-network
parameters can be specified statically for most or all different simulated
systems.
Diffs
-----
configs/ruby/Ruby.py d02b45a554b5
src/mem/protocol/RubySlicc_Defines.sm d02b45a554b5
src/mem/ruby/SConscript d02b45a554b5
src/mem/ruby/network/MessageBuffer.hh d02b45a554b5
src/mem/ruby/network/MessageBuffer.cc d02b45a554b5
src/mem/ruby/network/MessageBuffer.py PRE-CREATION
src/mem/ruby/network/SConscript d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.hh d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.cc d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.py d02b45a554b5
src/mem/ruby/network/simple/Switch.hh d02b45a554b5
src/mem/ruby/network/simple/Switch.cc d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.hh d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.cc d02b45a554b5
src/mem/slicc/ast/ObjDeclAST.py d02b45a554b5
src/mem/slicc/symbols/StateMachine.py d02b45a554b5
src/python/swig/pyobject.cc d02b45a554b5
Diff: http://reviews.gem5.org/r/2845/diff/
Testing
-------
Regression tests with all Ruby protocols. Protocol file changes are in
separate patch (http://reviews.gem5.org/r/2844/) to keep this one slim.
Thanks,
Joel Hestness
Nilay Vaish
2015-06-27 16:58:52 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2845/#review6628
-----------------------------------------------------------


Please post a snippet of the config.ini file generated after this patch is applied.


src/mem/ruby/network/MessageBuffer.hh (line 104)
<http://reviews.gem5.org/r/2845/#comment5721>

Drop these two functions and the m_description variable. The name of the message buffer should be enough information.



src/mem/ruby/network/MessageBuffer.py (line 38)
<http://reviews.gem5.org/r/2845/#comment5720>

Mention that 0 means infinite buffer size.


- Nilay Vaish
Post by Joel Hestness
-----------------------------------------------------------
http://reviews.gem5.org/r/2845/
-----------------------------------------------------------
(Updated June 20, 2015, 4:14 p.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 10859:d0dddb8fc9de
---------------------------
ruby: Expose MessageBuffers as SimObjects
Expose MessageBuffers from SLICC controllers as SimObjects that can be
1) First and foremost, it exposes MessageBuffers as SimObjects that can be
manipulated in Python code. This allows parameters to be set and checked in
Python code to avoid obfuscating parameters within protocol files. Further, now
as SimObjects, MessageBuffer parameters are printed to config output files as a
way to track parameters across simulations (e.g. buffer sizes)
2) Cleans up special-case code for responseFromMemory buffers, and aligns their
instantiation and use with mandatoryQueue buffers. These two special buffers
are the only MessageBuffers that are exposed to components outside of SLICC
controllers, and they're both slave ends of these buffers. They should be
exposed outside of SLICC in the same way, and this patch does it.
3) Distinguishes buffer-specific parameters from buffer-to-network parameters.
Specifically, buffer size, randomization, ordering, recycle latency, and ports
are all specific to a MessageBuffer, while the virtual network ID and type are
intrinsics of how the buffer is connected to network ports. The former are
specified in the Python object, while the latter are specified in the
controller *.sm files. Unlike buffer-specific parameters, which may need to
change depending on the simulated system structure, buffer-to-network
parameters can be specified statically for most or all different simulated
systems.
Diffs
-----
configs/ruby/Ruby.py d02b45a554b5
src/mem/protocol/RubySlicc_Defines.sm d02b45a554b5
src/mem/ruby/SConscript d02b45a554b5
src/mem/ruby/network/MessageBuffer.hh d02b45a554b5
src/mem/ruby/network/MessageBuffer.cc d02b45a554b5
src/mem/ruby/network/MessageBuffer.py PRE-CREATION
src/mem/ruby/network/SConscript d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.hh d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.cc d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.py d02b45a554b5
src/mem/ruby/network/simple/Switch.hh d02b45a554b5
src/mem/ruby/network/simple/Switch.cc d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.hh d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.cc d02b45a554b5
src/mem/slicc/ast/ObjDeclAST.py d02b45a554b5
src/mem/slicc/symbols/StateMachine.py d02b45a554b5
src/python/swig/pyobject.cc d02b45a554b5
Diff: http://reviews.gem5.org/r/2845/diff/
Testing
-------
Regression tests with all Ruby protocols. Protocol file changes are in
separate patch (http://reviews.gem5.org/r/2844/) to keep this one slim.
Thanks,
Joel Hestness
Joel Hestness
2015-07-24 23:41:26 UTC
Permalink
Post by Nilay Vaish
Please post a snippet of the config.ini file generated after this patch is applied.
[system.ruby.l1_cntrl0.mandatoryQueue]
type=MessageBuffer
buffer_size=0
eventq_index=0
ordered=false
randomization=false
recycle_latency=10
[system.ruby.l1_cntrl0.responseFromL1Cache]
type=MessageBuffer
buffer_size=0
eventq_index=0
ordered=false
randomization=false
recycle_latency=10
master=system.ruby.network.slave[1]
[system.ruby.l1_cntrl0.responseToL1Cache]
type=MessageBuffer
buffer_size=0
eventq_index=0
ordered=false
randomization=false
recycle_latency=10
slave=system.ruby.network.master[1]
[system.ruby.network.int_link_buffers01]
type=MessageBuffer
buffer_size=0
eventq_index=0
ordered=true
randomization=false
recycle_latency=0
- Joel


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2845/#review6628
-----------------------------------------------------------
-----------------------------------------------------------
http://reviews.gem5.org/r/2845/
-----------------------------------------------------------
(Updated July 24, 2015, 11:30 p.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 10936:2c16b97ba251
---------------------------
ruby: Expose MessageBuffers as SimObjects
Expose MessageBuffers from SLICC controllers as SimObjects that can be
1) First and foremost, it exposes MessageBuffers as SimObjects that can be
manipulated in Python code. This allows parameters to be set and checked in
Python code to avoid obfuscating parameters within protocol files. Further, now
as SimObjects, MessageBuffer parameters are printed to config output files as a
way to track parameters across simulations (e.g. buffer sizes)
2) Cleans up special-case code for responseFromMemory buffers, and aligns their
instantiation and use with mandatoryQueue buffers. These two special buffers
are the only MessageBuffers that are exposed to components outside of SLICC
controllers, and they're both slave ends of these buffers. They should be
exposed outside of SLICC in the same way, and this patch does it.
3) Distinguishes buffer-specific parameters from buffer-to-network parameters.
Specifically, buffer size, randomization, ordering, recycle latency, and ports
are all specific to a MessageBuffer, while the virtual network ID and type are
intrinsics of how the buffer is connected to network ports. The former are
specified in the Python object, while the latter are specified in the
controller *.sm files. Unlike buffer-specific parameters, which may need to
change depending on the simulated system structure, buffer-to-network
parameters can be specified statically for most or all different simulated
systems.
Diffs
-----
src/mem/ruby/network/SConscript cafae9abd4e4
src/mem/ruby/network/simple/PerfectSwitch.cc cafae9abd4e4
src/mem/ruby/network/simple/SimpleNetwork.hh cafae9abd4e4
src/mem/ruby/network/simple/SimpleNetwork.cc cafae9abd4e4
src/mem/ruby/network/simple/SimpleNetwork.py cafae9abd4e4
src/mem/ruby/network/simple/Switch.hh cafae9abd4e4
src/mem/ruby/network/simple/Switch.cc cafae9abd4e4
src/mem/ruby/network/simple/Throttle.cc cafae9abd4e4
src/mem/ruby/slicc_interface/AbstractController.hh cafae9abd4e4
src/mem/ruby/slicc_interface/AbstractController.cc cafae9abd4e4
configs/ruby/Ruby.py cafae9abd4e4
src/mem/protocol/RubySlicc_Defines.sm cafae9abd4e4
src/mem/ruby/SConscript cafae9abd4e4
src/mem/ruby/network/MessageBuffer.hh cafae9abd4e4
src/mem/ruby/network/MessageBuffer.cc cafae9abd4e4
src/mem/ruby/network/MessageBuffer.py PRE-CREATION
src/mem/slicc/ast/ObjDeclAST.py cafae9abd4e4
src/mem/slicc/symbols/StateMachine.py cafae9abd4e4
src/python/swig/pyobject.cc cafae9abd4e4
Diff: http://reviews.gem5.org/r/2845/diff/
Testing
-------
Regression tests with all Ruby protocols. Protocol file changes are in
separate patch (http://reviews.gem5.org/r/2844/) to keep this one slim.
Thanks,
Joel Hestness
Brad Beckmann
2015-07-22 17:10:34 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2845/#review6811
-----------------------------------------------------------


Joel, your intention with this patch is to completely replace patches http://reviews.gem5.org/r/2801/ and http://reviews.gem5.org/r/2814/, correct?

Hopefully we can resolve the last couple of issues on this patch and you can check it in soon. I'd like to add the finite buffer configs on top of it, as well as simple way to fallback on infinite buffers, similar to the kill switch added by 2801.

Thanks

- Brad Beckmann
Post by Joel Hestness
-----------------------------------------------------------
http://reviews.gem5.org/r/2845/
-----------------------------------------------------------
(Updated June 20, 2015, 4:14 p.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 10859:d0dddb8fc9de
---------------------------
ruby: Expose MessageBuffers as SimObjects
Expose MessageBuffers from SLICC controllers as SimObjects that can be
1) First and foremost, it exposes MessageBuffers as SimObjects that can be
manipulated in Python code. This allows parameters to be set and checked in
Python code to avoid obfuscating parameters within protocol files. Further, now
as SimObjects, MessageBuffer parameters are printed to config output files as a
way to track parameters across simulations (e.g. buffer sizes)
2) Cleans up special-case code for responseFromMemory buffers, and aligns their
instantiation and use with mandatoryQueue buffers. These two special buffers
are the only MessageBuffers that are exposed to components outside of SLICC
controllers, and they're both slave ends of these buffers. They should be
exposed outside of SLICC in the same way, and this patch does it.
3) Distinguishes buffer-specific parameters from buffer-to-network parameters.
Specifically, buffer size, randomization, ordering, recycle latency, and ports
are all specific to a MessageBuffer, while the virtual network ID and type are
intrinsics of how the buffer is connected to network ports. The former are
specified in the Python object, while the latter are specified in the
controller *.sm files. Unlike buffer-specific parameters, which may need to
change depending on the simulated system structure, buffer-to-network
parameters can be specified statically for most or all different simulated
systems.
Diffs
-----
configs/ruby/Ruby.py d02b45a554b5
src/mem/protocol/RubySlicc_Defines.sm d02b45a554b5
src/mem/ruby/SConscript d02b45a554b5
src/mem/ruby/network/MessageBuffer.hh d02b45a554b5
src/mem/ruby/network/MessageBuffer.cc d02b45a554b5
src/mem/ruby/network/MessageBuffer.py PRE-CREATION
src/mem/ruby/network/SConscript d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.hh d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.cc d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.py d02b45a554b5
src/mem/ruby/network/simple/Switch.hh d02b45a554b5
src/mem/ruby/network/simple/Switch.cc d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.hh d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.cc d02b45a554b5
src/mem/slicc/ast/ObjDeclAST.py d02b45a554b5
src/mem/slicc/symbols/StateMachine.py d02b45a554b5
src/python/swig/pyobject.cc d02b45a554b5
Diff: http://reviews.gem5.org/r/2845/diff/
Testing
-------
Regression tests with all Ruby protocols. Protocol file changes are in
separate patch (http://reviews.gem5.org/r/2844/) to keep this one slim.
Thanks,
Joel Hestness
Joel Hestness
2015-07-24 23:36:47 UTC
Permalink
Post by Joel Hestness
Post by Brad Beckmann
Joel, your intention with this patch is to completely replace patches http://reviews.gem5.org/r/2801/ and http://reviews.gem5.org/r/2814/, correct?
Hopefully we can resolve the last couple of issues on this patch and you can check it in soon. I'd like to add the finite buffer configs on top of it, as well as simple way to fallback on infinite buffers, similar to the kill switch added by 2801.
Thanks
Hi Brad. I believe this patch does everything that http://reviews.gem5.org/r/2801/ and http://reviews.gem5.org/r/2814/ aimed to provide (but configurable in Python).

I've updated this patch with your remaining feedback and reposted it.


- Joel


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2845/#review6811
-----------------------------------------------------------
Post by Joel Hestness
-----------------------------------------------------------
http://reviews.gem5.org/r/2845/
-----------------------------------------------------------
(Updated July 24, 2015, 11:30 p.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 10936:2c16b97ba251
---------------------------
ruby: Expose MessageBuffers as SimObjects
Expose MessageBuffers from SLICC controllers as SimObjects that can be
1) First and foremost, it exposes MessageBuffers as SimObjects that can be
manipulated in Python code. This allows parameters to be set and checked in
Python code to avoid obfuscating parameters within protocol files. Further, now
as SimObjects, MessageBuffer parameters are printed to config output files as a
way to track parameters across simulations (e.g. buffer sizes)
2) Cleans up special-case code for responseFromMemory buffers, and aligns their
instantiation and use with mandatoryQueue buffers. These two special buffers
are the only MessageBuffers that are exposed to components outside of SLICC
controllers, and they're both slave ends of these buffers. They should be
exposed outside of SLICC in the same way, and this patch does it.
3) Distinguishes buffer-specific parameters from buffer-to-network parameters.
Specifically, buffer size, randomization, ordering, recycle latency, and ports
are all specific to a MessageBuffer, while the virtual network ID and type are
intrinsics of how the buffer is connected to network ports. The former are
specified in the Python object, while the latter are specified in the
controller *.sm files. Unlike buffer-specific parameters, which may need to
change depending on the simulated system structure, buffer-to-network
parameters can be specified statically for most or all different simulated
systems.
Diffs
-----
src/mem/ruby/network/SConscript cafae9abd4e4
src/mem/ruby/network/simple/PerfectSwitch.cc cafae9abd4e4
src/mem/ruby/network/simple/SimpleNetwork.hh cafae9abd4e4
src/mem/ruby/network/simple/SimpleNetwork.cc cafae9abd4e4
src/mem/ruby/network/simple/SimpleNetwork.py cafae9abd4e4
src/mem/ruby/network/simple/Switch.hh cafae9abd4e4
src/mem/ruby/network/simple/Switch.cc cafae9abd4e4
src/mem/ruby/network/simple/Throttle.cc cafae9abd4e4
src/mem/ruby/slicc_interface/AbstractController.hh cafae9abd4e4
src/mem/ruby/slicc_interface/AbstractController.cc cafae9abd4e4
configs/ruby/Ruby.py cafae9abd4e4
src/mem/protocol/RubySlicc_Defines.sm cafae9abd4e4
src/mem/ruby/SConscript cafae9abd4e4
src/mem/ruby/network/MessageBuffer.hh cafae9abd4e4
src/mem/ruby/network/MessageBuffer.cc cafae9abd4e4
src/mem/ruby/network/MessageBuffer.py PRE-CREATION
src/mem/slicc/ast/ObjDeclAST.py cafae9abd4e4
src/mem/slicc/symbols/StateMachine.py cafae9abd4e4
src/python/swig/pyobject.cc cafae9abd4e4
Diff: http://reviews.gem5.org/r/2845/diff/
Testing
-------
Regression tests with all Ruby protocols. Protocol file changes are in
separate patch (http://reviews.gem5.org/r/2844/) to keep this one slim.
Thanks,
Joel Hestness
Joel Hestness
2015-07-24 23:30:07 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2845/
-----------------------------------------------------------

(Updated July 24, 2015, 11:30 p.m.)


Review request for Default.


Repository: gem5


Description (updated)
-------

Changeset 10936:2c16b97ba251
---------------------------
ruby: Expose MessageBuffers as SimObjects

Expose MessageBuffers from SLICC controllers as SimObjects that can be
manipulated in Python. This patch has numerous benefits:
1) First and foremost, it exposes MessageBuffers as SimObjects that can be
manipulated in Python code. This allows parameters to be set and checked in
Python code to avoid obfuscating parameters within protocol files. Further, now
as SimObjects, MessageBuffer parameters are printed to config output files as a
way to track parameters across simulations (e.g. buffer sizes)

2) Cleans up special-case code for responseFromMemory buffers, and aligns their
instantiation and use with mandatoryQueue buffers. These two special buffers
are the only MessageBuffers that are exposed to components outside of SLICC
controllers, and they're both slave ends of these buffers. They should be
exposed outside of SLICC in the same way, and this patch does it.

3) Distinguishes buffer-specific parameters from buffer-to-network parameters.
Specifically, buffer size, randomization, ordering, recycle latency, and ports
are all specific to a MessageBuffer, while the virtual network ID and type are
intrinsics of how the buffer is connected to network ports. The former are
specified in the Python object, while the latter are specified in the
controller *.sm files. Unlike buffer-specific parameters, which may need to
change depending on the simulated system structure, buffer-to-network
parameters can be specified statically for most or all different simulated
systems.


Diffs (updated)
-----

src/mem/ruby/network/SConscript cafae9abd4e4
src/mem/ruby/network/simple/PerfectSwitch.cc cafae9abd4e4
src/mem/ruby/network/simple/SimpleNetwork.hh cafae9abd4e4
src/mem/ruby/network/simple/SimpleNetwork.cc cafae9abd4e4
src/mem/ruby/network/simple/SimpleNetwork.py cafae9abd4e4
src/mem/ruby/network/simple/Switch.hh cafae9abd4e4
src/mem/ruby/network/simple/Switch.cc cafae9abd4e4
src/mem/ruby/network/simple/Throttle.cc cafae9abd4e4
src/mem/ruby/slicc_interface/AbstractController.hh cafae9abd4e4
src/mem/ruby/slicc_interface/AbstractController.cc cafae9abd4e4
configs/ruby/Ruby.py cafae9abd4e4
src/mem/protocol/RubySlicc_Defines.sm cafae9abd4e4
src/mem/ruby/SConscript cafae9abd4e4
src/mem/ruby/network/MessageBuffer.hh cafae9abd4e4
src/mem/ruby/network/MessageBuffer.cc cafae9abd4e4
src/mem/ruby/network/MessageBuffer.py PRE-CREATION
src/mem/slicc/ast/ObjDeclAST.py cafae9abd4e4
src/mem/slicc/symbols/StateMachine.py cafae9abd4e4
src/python/swig/pyobject.cc cafae9abd4e4

Diff: http://reviews.gem5.org/r/2845/diff/


Testing
-------

Regression tests with all Ruby protocols. Protocol file changes are in
separate patch (http://reviews.gem5.org/r/2844/) to keep this one slim.


Thanks,

Joel Hestness
Joel Hestness
2015-07-25 15:45:26 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2845/
-----------------------------------------------------------

(Updated July 25, 2015, 3:45 p.m.)


Review request for Default.


Repository: gem5


Description (updated)
-------

Changeset 10936:85b34fa1e87a
---------------------------
ruby: Expose MessageBuffers as SimObjects

Expose MessageBuffers from SLICC controllers as SimObjects that can be
manipulated in Python. This patch has numerous benefits:
1) First and foremost, it exposes MessageBuffers as SimObjects that can be
manipulated in Python code. This allows parameters to be set and checked in
Python code to avoid obfuscating parameters within protocol files. Further, now
as SimObjects, MessageBuffer parameters are printed to config output files as a
way to track parameters across simulations (e.g. buffer sizes)

2) Cleans up special-case code for responseFromMemory buffers, and aligns their
instantiation and use with mandatoryQueue buffers. These two special buffers
are the only MessageBuffers that are exposed to components outside of SLICC
controllers, and they're both slave ends of these buffers. They should be
exposed outside of SLICC in the same way, and this patch does it.

3) Distinguishes buffer-specific parameters from buffer-to-network parameters.
Specifically, buffer size, randomization, ordering, recycle latency, and ports
are all specific to a MessageBuffer, while the virtual network ID and type are
intrinsics of how the buffer is connected to network ports. The former are
specified in the Python object, while the latter are specified in the
controller *.sm files. Unlike buffer-specific parameters, which may need to
change depending on the simulated system structure, buffer-to-network
parameters can be specified statically for most or all different simulated
systems.


Diffs (updated)
-----

src/mem/ruby/network/simple/Switch.hh cafae9abd4e4
src/mem/ruby/network/simple/Switch.cc cafae9abd4e4
src/mem/ruby/network/simple/Throttle.cc cafae9abd4e4
src/mem/ruby/slicc_interface/AbstractController.hh cafae9abd4e4
src/mem/ruby/slicc_interface/AbstractController.cc cafae9abd4e4
src/mem/slicc/ast/ObjDeclAST.py cafae9abd4e4
src/mem/slicc/symbols/StateMachine.py cafae9abd4e4
src/python/swig/pyobject.cc cafae9abd4e4
src/mem/ruby/network/simple/SimpleNetwork.py cafae9abd4e4
src/mem/ruby/network/simple/SimpleNetwork.cc cafae9abd4e4
src/mem/ruby/network/simple/SimpleNetwork.hh cafae9abd4e4
src/mem/ruby/network/MessageBuffer.py PRE-CREATION
src/mem/ruby/network/SConscript cafae9abd4e4
src/mem/ruby/network/simple/PerfectSwitch.cc cafae9abd4e4
src/mem/ruby/network/MessageBuffer.cc cafae9abd4e4
configs/ruby/Ruby.py cafae9abd4e4
src/mem/protocol/RubySlicc_Defines.sm cafae9abd4e4
src/mem/ruby/SConscript cafae9abd4e4
src/mem/ruby/network/MessageBuffer.hh cafae9abd4e4

Diff: http://reviews.gem5.org/r/2845/diff/


Testing
-------

Regression tests with all Ruby protocols. Protocol file changes are in
separate patch (http://reviews.gem5.org/r/2844/) to keep this one slim.


Thanks,

Joel Hestness
Nilay Vaish
2015-07-27 22:29:59 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2845/#review6835
-----------------------------------------------------------



src/mem/ruby/SConscript (line 40)
<http://reviews.gem5.org/r/2845/#comment5897>

The flag RubyQueue is in use in MessageBuffer.cc. I think we should not add this one.



src/mem/ruby/network/MessageBuffer.hh (line 170)
<http://reviews.gem5.org/r/2845/#comment5900>

const



src/mem/ruby/network/MessageBuffer.hh (line 187)
<http://reviews.gem5.org/r/2845/#comment5901>

const



src/mem/ruby/network/MessageBuffer.hh (line 188)
<http://reviews.gem5.org/r/2845/#comment5899>

const since we are removing the set function.



src/mem/ruby/network/MessageBuffer.cc (lines 51 - 54)
<http://reviews.gem5.org/r/2845/#comment5902>

Let's mark these members are const and have them initialized using initializer list.



src/mem/ruby/network/MessageBuffer.py (line 1)
<http://reviews.gem5.org/r/2845/#comment5896>

2015 Mark D. Hill and David A. Wood



src/mem/ruby/network/simple/SimpleNetwork.py (lines 48 - 77)
<http://reviews.gem5.org/r/2845/#comment5903>

I read Brad's comment on this function. I think the original choice was right. I don't see why we want to compile this into the binary. If configs/ruby/Ruby.py is becoming a problem, we should just split that file.



src/mem/slicc/ast/ObjDeclAST.py (lines 50 - 54)
<http://reviews.gem5.org/r/2845/#comment5904>

Do we need this code?



src/python/swig/pyobject.cc (line 109)
<http://reviews.gem5.org/r/2845/#comment5905>

Can you confirm that these checks on name1 and name2 are still required?


- Nilay Vaish
Post by Joel Hestness
-----------------------------------------------------------
http://reviews.gem5.org/r/2845/
-----------------------------------------------------------
(Updated July 25, 2015, 3:45 p.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 10936:85b34fa1e87a
---------------------------
ruby: Expose MessageBuffers as SimObjects
Expose MessageBuffers from SLICC controllers as SimObjects that can be
1) First and foremost, it exposes MessageBuffers as SimObjects that can be
manipulated in Python code. This allows parameters to be set and checked in
Python code to avoid obfuscating parameters within protocol files. Further, now
as SimObjects, MessageBuffer parameters are printed to config output files as a
way to track parameters across simulations (e.g. buffer sizes)
2) Cleans up special-case code for responseFromMemory buffers, and aligns their
instantiation and use with mandatoryQueue buffers. These two special buffers
are the only MessageBuffers that are exposed to components outside of SLICC
controllers, and they're both slave ends of these buffers. They should be
exposed outside of SLICC in the same way, and this patch does it.
3) Distinguishes buffer-specific parameters from buffer-to-network parameters.
Specifically, buffer size, randomization, ordering, recycle latency, and ports
are all specific to a MessageBuffer, while the virtual network ID and type are
intrinsics of how the buffer is connected to network ports. The former are
specified in the Python object, while the latter are specified in the
controller *.sm files. Unlike buffer-specific parameters, which may need to
change depending on the simulated system structure, buffer-to-network
parameters can be specified statically for most or all different simulated
systems.
Diffs
-----
src/mem/ruby/network/simple/Switch.hh cafae9abd4e4
src/mem/ruby/network/simple/Switch.cc cafae9abd4e4
src/mem/ruby/network/simple/Throttle.cc cafae9abd4e4
src/mem/ruby/slicc_interface/AbstractController.hh cafae9abd4e4
src/mem/ruby/slicc_interface/AbstractController.cc cafae9abd4e4
src/mem/slicc/ast/ObjDeclAST.py cafae9abd4e4
src/mem/slicc/symbols/StateMachine.py cafae9abd4e4
src/python/swig/pyobject.cc cafae9abd4e4
src/mem/ruby/network/simple/SimpleNetwork.py cafae9abd4e4
src/mem/ruby/network/simple/SimpleNetwork.cc cafae9abd4e4
src/mem/ruby/network/simple/SimpleNetwork.hh cafae9abd4e4
src/mem/ruby/network/MessageBuffer.py PRE-CREATION
src/mem/ruby/network/SConscript cafae9abd4e4
src/mem/ruby/network/simple/PerfectSwitch.cc cafae9abd4e4
src/mem/ruby/network/MessageBuffer.cc cafae9abd4e4
configs/ruby/Ruby.py cafae9abd4e4
src/mem/protocol/RubySlicc_Defines.sm cafae9abd4e4
src/mem/ruby/SConscript cafae9abd4e4
src/mem/ruby/network/MessageBuffer.hh cafae9abd4e4
Diff: http://reviews.gem5.org/r/2845/diff/
Testing
-------
Regression tests with all Ruby protocols. Protocol file changes are in
separate patch (http://reviews.gem5.org/r/2844/) to keep this one slim.
Thanks,
Joel Hestness
Brad Beckmann
2015-07-27 23:12:33 UTC
Permalink
Post by Joel Hestness
src/mem/ruby/network/simple/SimpleNetwork.py, lines 48-77
<http://reviews.gem5.org/r/2845/diff/6/?file=48492#file48492line48>
I read Brad's comment on this function. I think the original choice was right. I don't see why we want to compile this into the binary. If configs/ruby/Ruby.py is becoming a problem, we should just split that file.
The function manipulates members of the network. It belongs in the network class. That is simply good object-oriented design.


- Brad


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2845/#review6835
-----------------------------------------------------------
Post by Joel Hestness
-----------------------------------------------------------
http://reviews.gem5.org/r/2845/
-----------------------------------------------------------
(Updated July 25, 2015, 3:45 p.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 10936:85b34fa1e87a
---------------------------
ruby: Expose MessageBuffers as SimObjects
Expose MessageBuffers from SLICC controllers as SimObjects that can be
1) First and foremost, it exposes MessageBuffers as SimObjects that can be
manipulated in Python code. This allows parameters to be set and checked in
Python code to avoid obfuscating parameters within protocol files. Further, now
as SimObjects, MessageBuffer parameters are printed to config output files as a
way to track parameters across simulations (e.g. buffer sizes)
2) Cleans up special-case code for responseFromMemory buffers, and aligns their
instantiation and use with mandatoryQueue buffers. These two special buffers
are the only MessageBuffers that are exposed to components outside of SLICC
controllers, and they're both slave ends of these buffers. They should be
exposed outside of SLICC in the same way, and this patch does it.
3) Distinguishes buffer-specific parameters from buffer-to-network parameters.
Specifically, buffer size, randomization, ordering, recycle latency, and ports
are all specific to a MessageBuffer, while the virtual network ID and type are
intrinsics of how the buffer is connected to network ports. The former are
specified in the Python object, while the latter are specified in the
controller *.sm files. Unlike buffer-specific parameters, which may need to
change depending on the simulated system structure, buffer-to-network
parameters can be specified statically for most or all different simulated
systems.
Diffs
-----
src/mem/ruby/network/simple/Switch.hh cafae9abd4e4
src/mem/ruby/network/simple/Switch.cc cafae9abd4e4
src/mem/ruby/network/simple/Throttle.cc cafae9abd4e4
src/mem/ruby/slicc_interface/AbstractController.hh cafae9abd4e4
src/mem/ruby/slicc_interface/AbstractController.cc cafae9abd4e4
src/mem/slicc/ast/ObjDeclAST.py cafae9abd4e4
src/mem/slicc/symbols/StateMachine.py cafae9abd4e4
src/python/swig/pyobject.cc cafae9abd4e4
src/mem/ruby/network/simple/SimpleNetwork.py cafae9abd4e4
src/mem/ruby/network/simple/SimpleNetwork.cc cafae9abd4e4
src/mem/ruby/network/simple/SimpleNetwork.hh cafae9abd4e4
src/mem/ruby/network/MessageBuffer.py PRE-CREATION
src/mem/ruby/network/SConscript cafae9abd4e4
src/mem/ruby/network/simple/PerfectSwitch.cc cafae9abd4e4
src/mem/ruby/network/MessageBuffer.cc cafae9abd4e4
configs/ruby/Ruby.py cafae9abd4e4
src/mem/protocol/RubySlicc_Defines.sm cafae9abd4e4
src/mem/ruby/SConscript cafae9abd4e4
src/mem/ruby/network/MessageBuffer.hh cafae9abd4e4
Diff: http://reviews.gem5.org/r/2845/diff/
Testing
-------
Regression tests with all Ruby protocols. Protocol file changes are in
separate patch (http://reviews.gem5.org/r/2844/) to keep this one slim.
Thanks,
Joel Hestness
Joel Hestness
2015-07-31 14:18:28 UTC
Permalink
Post by Brad Beckmann
src/mem/slicc/ast/ObjDeclAST.py, lines 60-64
<http://reviews.gem5.org/r/2845/diff/6/?file=48498#file48498line60>
Do we need this code?
No. I've removed this.
Post by Brad Beckmann
src/mem/ruby/network/simple/SimpleNetwork.py, lines 48-77
<http://reviews.gem5.org/r/2845/diff/6/?file=48492#file48492line48>
I read Brad's comment on this function. I think the original choice was right. I don't see why we want to compile this into the binary. If configs/ruby/Ruby.py is becoming a problem, we should just split that file.
The function manipulates members of the network. It belongs in the network class. That is simply good object-oriented design.
I've felt the same way as Nilay about this, but followed Brad's suggestion in hopes of getting this patch out the door.

This code can be quite problematic to leave in SimpleNetwork.py and compile into the binary: Suppose someone wanted to set the buffer size or randomization on these buffers, which seem like a reasonable ideas. Either they would have to modify this code and recompile the simulator, or they would have to hack up Ruby.py\/another config file to (a) reinstantiate all these buffers with appropriate parameters, (b) append them in local lists, and (c) reassign the network and switch vector params with those lists. Yes, having this in SimpleNetwork is good object-oriented design, but it is likely to cause headaches and kinda defeats the purpose of having simulation configs.

@Brad: Are you strongly opposed to housing this in Ruby.py?
Post by Brad Beckmann
src/python/swig/pyobject.cc, line 109
<http://reviews.gem5.org/r/2845/diff/6/?file=48500#file48500line109>
Can you confirm that these checks on name1 and name2 are still required?
Seems these are no longer required, since we are now casting to MessageBuffers instead of AbstractControllers and SLICC generated code now connects buffers to memories. I've removed these checks.


- Joel


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2845/#review6835
-----------------------------------------------------------
Post by Brad Beckmann
-----------------------------------------------------------
http://reviews.gem5.org/r/2845/
-----------------------------------------------------------
(Updated July 25, 2015, 3:45 p.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 10936:85b34fa1e87a
---------------------------
ruby: Expose MessageBuffers as SimObjects
Expose MessageBuffers from SLICC controllers as SimObjects that can be
1) First and foremost, it exposes MessageBuffers as SimObjects that can be
manipulated in Python code. This allows parameters to be set and checked in
Python code to avoid obfuscating parameters within protocol files. Further, now
as SimObjects, MessageBuffer parameters are printed to config output files as a
way to track parameters across simulations (e.g. buffer sizes)
2) Cleans up special-case code for responseFromMemory buffers, and aligns their
instantiation and use with mandatoryQueue buffers. These two special buffers
are the only MessageBuffers that are exposed to components outside of SLICC
controllers, and they're both slave ends of these buffers. They should be
exposed outside of SLICC in the same way, and this patch does it.
3) Distinguishes buffer-specific parameters from buffer-to-network parameters.
Specifically, buffer size, randomization, ordering, recycle latency, and ports
are all specific to a MessageBuffer, while the virtual network ID and type are
intrinsics of how the buffer is connected to network ports. The former are
specified in the Python object, while the latter are specified in the
controller *.sm files. Unlike buffer-specific parameters, which may need to
change depending on the simulated system structure, buffer-to-network
parameters can be specified statically for most or all different simulated
systems.
Diffs
-----
src/mem/ruby/network/simple/Switch.hh cafae9abd4e4
src/mem/ruby/network/simple/Switch.cc cafae9abd4e4
src/mem/ruby/network/simple/Throttle.cc cafae9abd4e4
src/mem/ruby/slicc_interface/AbstractController.hh cafae9abd4e4
src/mem/ruby/slicc_interface/AbstractController.cc cafae9abd4e4
src/mem/slicc/ast/ObjDeclAST.py cafae9abd4e4
src/mem/slicc/symbols/StateMachine.py cafae9abd4e4
src/python/swig/pyobject.cc cafae9abd4e4
src/mem/ruby/network/simple/SimpleNetwork.py cafae9abd4e4
src/mem/ruby/network/simple/SimpleNetwork.cc cafae9abd4e4
src/mem/ruby/network/simple/SimpleNetwork.hh cafae9abd4e4
src/mem/ruby/network/MessageBuffer.py PRE-CREATION
src/mem/ruby/network/SConscript cafae9abd4e4
src/mem/ruby/network/simple/PerfectSwitch.cc cafae9abd4e4
src/mem/ruby/network/MessageBuffer.cc cafae9abd4e4
configs/ruby/Ruby.py cafae9abd4e4
src/mem/protocol/RubySlicc_Defines.sm cafae9abd4e4
src/mem/ruby/SConscript cafae9abd4e4
src/mem/ruby/network/MessageBuffer.hh cafae9abd4e4
Diff: http://reviews.gem5.org/r/2845/diff/
Testing
-------
Regression tests with all Ruby protocols. Protocol file changes are in
separate patch (http://reviews.gem5.org/r/2844/) to keep this one slim.
Thanks,
Joel Hestness
Brad Beckmann
2015-07-27 23:04:12 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2845/#review6836
-----------------------------------------------------------

Ship it!


Thanks Joel. This looks great to me!

Can you please check this in after we check in our patches on 7/31. We have discarded patches http://reviews.gem5.org/r/2801 and http://reviews.gem5.org/r/2814. We will build off of this patch instead.

- Brad Beckmann
Post by Joel Hestness
-----------------------------------------------------------
http://reviews.gem5.org/r/2845/
-----------------------------------------------------------
(Updated July 25, 2015, 3:45 p.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 10936:85b34fa1e87a
---------------------------
ruby: Expose MessageBuffers as SimObjects
Expose MessageBuffers from SLICC controllers as SimObjects that can be
1) First and foremost, it exposes MessageBuffers as SimObjects that can be
manipulated in Python code. This allows parameters to be set and checked in
Python code to avoid obfuscating parameters within protocol files. Further, now
as SimObjects, MessageBuffer parameters are printed to config output files as a
way to track parameters across simulations (e.g. buffer sizes)
2) Cleans up special-case code for responseFromMemory buffers, and aligns their
instantiation and use with mandatoryQueue buffers. These two special buffers
are the only MessageBuffers that are exposed to components outside of SLICC
controllers, and they're both slave ends of these buffers. They should be
exposed outside of SLICC in the same way, and this patch does it.
3) Distinguishes buffer-specific parameters from buffer-to-network parameters.
Specifically, buffer size, randomization, ordering, recycle latency, and ports
are all specific to a MessageBuffer, while the virtual network ID and type are
intrinsics of how the buffer is connected to network ports. The former are
specified in the Python object, while the latter are specified in the
controller *.sm files. Unlike buffer-specific parameters, which may need to
change depending on the simulated system structure, buffer-to-network
parameters can be specified statically for most or all different simulated
systems.
Diffs
-----
src/mem/ruby/network/simple/Switch.hh cafae9abd4e4
src/mem/ruby/network/simple/Switch.cc cafae9abd4e4
src/mem/ruby/network/simple/Throttle.cc cafae9abd4e4
src/mem/ruby/slicc_interface/AbstractController.hh cafae9abd4e4
src/mem/ruby/slicc_interface/AbstractController.cc cafae9abd4e4
src/mem/slicc/ast/ObjDeclAST.py cafae9abd4e4
src/mem/slicc/symbols/StateMachine.py cafae9abd4e4
src/python/swig/pyobject.cc cafae9abd4e4
src/mem/ruby/network/simple/SimpleNetwork.py cafae9abd4e4
src/mem/ruby/network/simple/SimpleNetwork.cc cafae9abd4e4
src/mem/ruby/network/simple/SimpleNetwork.hh cafae9abd4e4
src/mem/ruby/network/MessageBuffer.py PRE-CREATION
src/mem/ruby/network/SConscript cafae9abd4e4
src/mem/ruby/network/simple/PerfectSwitch.cc cafae9abd4e4
src/mem/ruby/network/MessageBuffer.cc cafae9abd4e4
configs/ruby/Ruby.py cafae9abd4e4
src/mem/protocol/RubySlicc_Defines.sm cafae9abd4e4
src/mem/ruby/SConscript cafae9abd4e4
src/mem/ruby/network/MessageBuffer.hh cafae9abd4e4
Diff: http://reviews.gem5.org/r/2845/diff/
Testing
-------
Regression tests with all Ruby protocols. Protocol file changes are in
separate patch (http://reviews.gem5.org/r/2844/) to keep this one slim.
Thanks,
Joel Hestness
Joel Hestness
2015-07-15 20:06:32 UTC
Permalink
Post by Joel Hestness
This patch looks very good to me. Small thing, you might want to consider renaming m_buffers_to_free; the name is not really applicable now.
Thanks. I changed these names.


- Joel


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2845/#review6480
-----------------------------------------------------------
Post by Joel Hestness
-----------------------------------------------------------
http://reviews.gem5.org/r/2845/
-----------------------------------------------------------
(Updated June 20, 2015, 4:14 p.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 10859:d0dddb8fc9de
---------------------------
ruby: Expose MessageBuffers as SimObjects
Expose MessageBuffers from SLICC controllers as SimObjects that can be
1) First and foremost, it exposes MessageBuffers as SimObjects that can be
manipulated in Python code. This allows parameters to be set and checked in
Python code to avoid obfuscating parameters within protocol files. Further, now
as SimObjects, MessageBuffer parameters are printed to config output files as a
way to track parameters across simulations (e.g. buffer sizes)
2) Cleans up special-case code for responseFromMemory buffers, and aligns their
instantiation and use with mandatoryQueue buffers. These two special buffers
are the only MessageBuffers that are exposed to components outside of SLICC
controllers, and they're both slave ends of these buffers. They should be
exposed outside of SLICC in the same way, and this patch does it.
3) Distinguishes buffer-specific parameters from buffer-to-network parameters.
Specifically, buffer size, randomization, ordering, recycle latency, and ports
are all specific to a MessageBuffer, while the virtual network ID and type are
intrinsics of how the buffer is connected to network ports. The former are
specified in the Python object, while the latter are specified in the
controller *.sm files. Unlike buffer-specific parameters, which may need to
change depending on the simulated system structure, buffer-to-network
parameters can be specified statically for most or all different simulated
systems.
Diffs
-----
configs/ruby/Ruby.py d02b45a554b5
src/mem/protocol/RubySlicc_Defines.sm d02b45a554b5
src/mem/ruby/SConscript d02b45a554b5
src/mem/ruby/network/MessageBuffer.hh d02b45a554b5
src/mem/ruby/network/MessageBuffer.cc d02b45a554b5
src/mem/ruby/network/MessageBuffer.py PRE-CREATION
src/mem/ruby/network/SConscript d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.hh d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.cc d02b45a554b5
src/mem/ruby/network/simple/SimpleNetwork.py d02b45a554b5
src/mem/ruby/network/simple/Switch.hh d02b45a554b5
src/mem/ruby/network/simple/Switch.cc d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.hh d02b45a554b5
src/mem/ruby/slicc_interface/AbstractController.cc d02b45a554b5
src/mem/slicc/ast/ObjDeclAST.py d02b45a554b5
src/mem/slicc/symbols/StateMachine.py d02b45a554b5
src/python/swig/pyobject.cc d02b45a554b5
Diff: http://reviews.gem5.org/r/2845/diff/
Testing
-------
Regression tests with all Ruby protocols. Protocol file changes are in
separate patch (http://reviews.gem5.org/r/2844/) to keep this one slim.
Thanks,
Joel Hestness
Loading...