Ruby: Remove CacheMsg class from SLICC
Review Request #327 - Created Dec. 1, 2010 and submitted
| Information | |
|---|---|
| Nilay Vaish | |
| gem5 | |
| default | |
| Reviewers | |
| Default | |
Ruby: Remove CacheMsg class from SLICC The goal of the patch is to do away with the CacheMsg class currently in use in coherence protocols. In place of CacheMsg, the RubyRequest class will used. This class is already present in slicc_interface/RubyRequest.hh. In fact, objects of class CacheMsg are generated by copying values from a RubyRequest object.
-
src/mem/ruby/system/Sequencer.cc (Diff revision 2) -
This is the portion of code that is not working correctly. If I compile this code, it works correctly with ruby random tester. But if I were to un-comment this line and comment out the one below, the code exits due to segmentation fault. In the debug mode, the code fails one of the assertion on the request type.
I've only had a chance to briefly review this, but I do have a couple comments: - The hammer cache changes didn't seem to upload cleanly. Can you try to post them again? - I just want to confirm that the libruby interface is still useful to you all at Wisconsin, correct? I just want to make sure I understand why the libruby files exist after your change. Is it possible to eliminate at least a few of these functions? For example the libruby_init() function that takes a configuration file name as an argument?
Diff: |
Revision 3 (+480 -532) |
|---|
Diff: |
Revision 4 (+500 -532) |
|---|
Description: |
|
||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Branch: |
|
||||||||||||||||||||||||||||||||||||
Diff: |
Revision 5 (+541 -897) |
Description: |
|
|||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 6 (+539 -560) |
Overall, this patch looks good to me as well. I just have a couple minor questions. Though your comment says this patch removes libruby, the diff seems to indicate that the file still remains. Am I reading that correctly? Also it seems that in several places you unecessarily generate the line address, even though the line address already exists in the ruby request (see comments below).
-
src/mem/ruby/storebuffer/storebuffer.cc (Diff revision 6) -
Instead of masking the physical address, can we just use the m_LineAddress?
-
src/mem/ruby/storebuffer/storebuffer.cc (Diff revision 6) -
Same here. Can we just use the m_LineAddress?
-
src/mem/ruby/storebuffer/storebuffer.cc (Diff revision 6) -
And here
-
src/mem/ruby/system/Sequencer.cc (Diff revision 6) -
Why is this line address conversion necessary? Isn't m_LineAddress already set correctly in the constructor?
-
src/mem/ruby/system/Sequencer.cc (Diff revision 6) -
Can you just use the m_LineAddress variable of ruby_request instead of converting the paddr to a line address.
Summary: |
|
|||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Description: |
|
|||||||||||||||||||||||||||||||||
Diff: |
Revision 7 (+207 -163) |