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
