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]> 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]] 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] > http://m5sim.org/mailman/listinfo/gem5-dev > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
