Oops I should have said the patch introduces the MachineType enum, rather than removes it. Please disregard the third paragraph in my prior email.
Yes, this patch helps with collecting stats that are protocol specific. Eventually we need to move the MachineType underneath the RubySystem. I think this patch will help in that effort. Brad From: Beckmann, Brad Sent: Friday, May 08, 2015 1:04 PM To: 'Jason Power'; gem5 Developer List; Nilay Vaish Subject: RE: [gem5-dev] Review Request 2551: ruby: slicc: have a static MachineType First off, I was also seeing the private patch error. I think Nilay changed the access permissions on that particular patch for a moment. It should now be accessible again. Let’s be clear, the MachineType object and its associated functions were static before this change. That is a big problem that we will need to fix soon. You’ll see more from Brandon Potter on this soon. I believe this patch actually is a step in the right direction as far as removing the static dependencies on MachineType. Before this patch, the MachineType enum needed to be inclusive of all possible machine types created by the protocols. Thus the RubySlicc_Export.sm file was a global file that needed to be modified by every protocol that introduced a new machine name. That dependency goes away with this change. Yes, we do have patches in the pipeline that implement non-CPU cache controllers. They depend on this patch being checked in. Brad From: Jason Power [mailto:[email protected]] Sent: Friday, May 08, 2015 11:57 AM To: gem5 Developer List; Beckmann, Brad; Nilay Vaish Subject: Re: [gem5-dev] Review Request 2551: ruby: slicc: have a static MachineType Hi Brad, I personally would not like to see this patch pushed as it is. It breaks our workflow for gem5-gpu. Also, it seems to me that making the MachineTypes static is a step backwards. It's already hard with Ruby to create a protocol that isn't a traditional CPU-oriented protocol, and by not allowing new machine types (easily) it makes it that much harder. I have to admit it's been a long time since I looked at this patch, and unfortunately I can't right now because I'm getting an error with reviewboard "This review request is private. You must be a requested reviewer, either directly or on a requested group, and have permission to access the repository in order to view this review request." Strange. Brad, do you have any patches in the (very near) pipeline that will allow for more MachineTypes than just the basic CPU-oriented caches? I suppose I could be convinced to modify our protocols once, but I really don't want to have to change them with every gem5 update more than we already do. Overall, I can't remember what problem this patch originally solves. Was the problem that the Ruby stats depended on a static number of (Generic)MachineTypes? If this is the problem this patch solves, wouldn't it be better to instead make the stats accept dynamic MachineTypes? If the rest of the community disagrees with me and wants this patch as is, we can deal here at gem5-gpu. I would just much rather keep the changes we have to the mainline gem5 minimal as merges are already pretty hellish as it is. Thanks, Jason On Fri, May 8, 2015 at 12:07 PM Beckmann, Brad <[email protected]<mailto:[email protected]>> wrote: Hi Nilay, Can you check this patch and the previous patch (review 2550) in the repo? We will be posting a series of patches next week and they depend on these two patches. Thanks, Brad From: Brad Beckmann [mailto:[email protected]<mailto:[email protected]>] On Behalf Of Beckmann, Brad Sent: Monday, April 27, 2015 3:30 PM To: Nilay Vaish; Beckmann, Brad; Default Subject: Re: Review Request 2551: ruby: slicc: have a static MachineType This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2551/ Ship it! Ship It! - Brad Beckmann On December 4th, 2014, 6:08 p.m. UTC, Nilay Vaish wrote: Review request for Default. By Nilay Vaish. Updated Dec. 4, 2014, 6:08 p.m. Repository: gem5 Description Changeset 10594:b25131489b95 --------------------------- ruby: slicc: have a static MachineType This patch moves from a dynamically defined MachineType to a statically defined one. The need for this patch was felt since a dynamically defined type prevents us from having types for which no machine definition may exist. The following changes have been made: i. each machine definition now uses a type from the MachineType enumeration instead of any random identifier. This required changing the grammar and the *.sm files. ii. MachineType enumeration defined statically in RubySlicc_Exports.sm. Diffs * src/mem/protocol/MESI_Two_Level-L1cache.sm (fea29fc045ee) * src/mem/protocol/MESI_Two_Level-L2cache.sm (fea29fc045ee) * src/mem/protocol/MESI_Two_Level-dir.sm (fea29fc045ee) * src/mem/protocol/MESI_Two_Level-dma.sm (fea29fc045ee) * src/mem/protocol/RubySlicc_Exports.sm (fea29fc045ee) * src/mem/slicc/ast/DeclListAST.py (fea29fc045ee) * src/mem/slicc/ast/MachineAST.py (fea29fc045ee) * src/mem/slicc/parser.py (fea29fc045ee) * src/mem/slicc/symbols/SymbolTable.py (fea29fc045ee) * src/mem/slicc/symbols/Type.py (fea29fc045ee) View Diff<http://reviews.gem5.org/r/2551/diff/> _______________________________________________ gem5-dev mailing list [email protected]<mailto:[email protected]> http://m5sim.org/mailman/listinfo/gem5-dev _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
