It has been a couple weeks since we discussed this patch. I hope that folks can see this patch in a different light.
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. 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". Thanks, Brad -----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
