Hey Brad, The definition of MachineType is not only needed for protocol specific > analysis within Ruby, but it needed for protocol specific mapping functions. > > I would like to point out that today, the function > map_Address_to_Directory in RubySlicc_ComponentMapping.hh requires that all > protocols define a Directory machine. By adding this patch, that > requirement will no longer be necessary. It will also allow folks to add > different protocol specific mapping functions without breaking all other > protocols. >
You are suggesting that one instance of this should merit changing the whole codebase - I disagree. You should note that the map_Address_to_Directory function is a convenience so that .sm files need not create the MachineID to send to a directory. It would be very straightforward to remove the reference to MachineType_Directory in RubySlicc_ComponentMapping.hh and make .sm files generate the MachineID; map_Address_to_Directory would just return the node ID. If someone wanted to create a snooping Ruby protocol, this would be a good first step. Further, there is significant precedent that gem5 has been moving toward MachineType independence in protocol-independent code. We removed the GenericMachineType, dynamically generate the MachineTypes enum, and removed references to protocol-specific MachineTypes in the sequencer. Let's be forward-thinking about the codebase rather than revisionist. Let's not insist that every feature added to Ruby must have a mainline > use. The public tree should prioritize being a useful baseline that others > can build on top of rather than trying to achieve "perfection". > I agree with you that the public tree should be easily extensible, but it should also prioritize guiding users toward appropriate use of the existing infrastructure. In this case, there are no necessary mainline uses and poorly understood non-mainline uses for protocol-specific MachineType handling in protocol-independent code. We have also discussed that protocol-specific profiling can be supported by adding stats in SLICC. Stats would be a far more extensible solution than allowing protocol-specific MachineTypes to permeate the codebase. It is in the community's interest to add stats to SLICC. There needs to be forcing functions to make these things happen somewhere. Joel -----Original Message----- > From: gem5-dev [mailto:[email protected]] On Behalf Of Joel > Hestness > Sent: Monday, May 11, 2015 11:15 AM > To: gem5 Developer List > Subject: Re: [gem5-dev] Review Request 2551: ruby: slicc: have a static > MachineType > > Hi guys, > More thoughts are inlined below: > > > Hi guys, > >> > >> @Nilay: Thanks for clarifying the desired MachineType usage. > >> > >> I understand the aims now, and I still disagree with statically > >> defined MachineTypes. SLICC is already very powerful, and it only > >> makes sense to add features, not contract them. In this case, scons > >> and SLICC already do the following for us: (1) they allow dynamically > >> extending the list of protocols in any SConscript ('all_protocols' > >> variable), (2) they traverse all directories to find all > >> protocol-related files, and (3) they generate all the MachineType code. > This is nearly everything we need! > >> > >> So, it seems that we're divided on whether we should "just make > >> MachineTypes work" or "do this the right way". I spent a little time > >> today to assemble a patch that dynamically generates the MachineType > >> enum from the superset of all protocol controller definitions. Here it > is: > >> http://reviews.gem5.org/r/2773/ > >> > >> Overall, I feel that the vision should be that SLICC be able to > >> generate and build any/all protocols in a single build operation > >> (like gem5's SE+FS mode merge). I suspect that direction will involve > >> eliminating statically defined types, since they often need to be > >> updated per-protocol, which tends to cause merge headaches. > >> > >> Joel > >> > >> > >> > > > > Joel, AMD might have used machine types for which no definition exists > > in any of the protocols. This used to happen in erstwhile > GenericMachineType. > > > > I'm not necessarily advocating for my patch, but we shouldn't allow code > that functions on non-existent types. Such would be dead code and doesn't > have a place in the mainline repo. Some GenericMachineTypes may have gone > unused, but only because they were statically defined. Dynamically defined > types would eliminate the existence of dead types. > > > I am not in favor of committing Joel's patch or the one I wrote (which is > > why I had taken it off the reviewboard). We should not add protocol > > specific code to files that are not protocol-specific. Such code > > should reside in protocol specific .sm files. If required, SLICC > > should be modified. I am unable to think of any technical difficulty > > that prevents us from adding statistical variables to .sm files. We > > already have cache hits and misses being collected in .sm files. > > > > I am not in favor of the argument that we should stick to same way as > > things were done in past. If in past something was done incorrectly / > > inefficiently / not-correct-completely, we should be willing to change > it. > > In the current context, protocol-specific code in protocol-independent > > files was not the right thing to do. Therefore, we should be > > unwilling to allow such code. > > > After reviewing this thread and the prior thread on protocol-specific > profiling (https://www.mail-archive.com/[email protected]/msg13084.html), > I agree with Nilay that we should disallow protocol-specific activity in > protocol-independent code. Currently we don't have any such activity in > mainline gem5, and we don't have a well-defined use case for it that we can > review. If we think there is a real need for it, I'd request that the code > be posted for us to review. Having specific use cases would be helpful in > deciding whether/how to manage it. > > To riff on Nilay's suggestions for modifying SLICC: It appears it would be > straightforward to add protocol-specific profiling within protocol files by > extending SLICC to add gem5 stats. While it would be more complex, it even > looks feasible to add enough machine inheritance that different protocols > could share common profiling functionality (we could also share other > things if we were so inclined). > > Joel > > > -- > Joel Hestness > PhD Candidate, Computer Architecture > Dept. of Computer Science, University of Wisconsin - Madison > http://pages.cs.wisc.edu/~hestness/ > _______________________________________________ > 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 > -- Joel Hestness PhD Candidate, Computer Architecture Dept. of Computer Science, University of Wisconsin - Madison http://pages.cs.wisc.edu/~hestness/ _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
