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
On Fri, May 8, 2015 at 8:09 PM, Nilay Vaish <[email protected]> wrote:
> On Fri, 8 May 2015, Joel Hestness wrote:
>
> Hi guys,
>> I have some input here:
>> 1) First and foremost: If we decide the direction of this patch is
>> alright, it should *NOT* ship as is, because it only fixes the machine
>> type
>> for the MESI_Two_Level protocol. The rest of the protocol files need to be
>> updated (unless I'm overlooking something).
>>
>> 2) I agree with Jason that we shouldn't have a statically defined set of
>> machine types. The way that Ruby generates controllers is very flexible so
>> that adding or swapping out controllers is simple. You would no longer be
>> able to do this without the MachineType enumeration containing a superset
>> of all the controllers that various protocols include.
>>
>> 3) I've read this request each time discussion has come up, and I still
>> don't understand the need for it. As is, the patch's aim and the
>> protocol-specific stats problem are not adequately described for this
>> patch
>> to make sense. I'd strongly prefer that the patch description be rewritten
>> so that it is much clearer what is meant by "prevents us from having types
>> for which no machine definition may exist". I'd also strongly prefer a
>> more
>> detailed description of the problem with protocol-specific stats. These
>> are
>> a must to offer an informed patch review.
>>
>>
>
> I think AMD has some code that looks like following:
>
> if (type == MachineTypeXXX)
> do YYY;
>
>
> This is in C++, and not in SLICC. So, you need that MachineTypeXXX is
> defined. MachineType enum, as of now, is defined dynamically. So, if you
> have two different protocols, then MachineType enum defined in one protocol
> would not be visible when the other protocol is compiled. But the C++ code
> expects that MachineTypeXXX is defined irrespective of the protocol. So,
> AMD is pitching for statically defined types, that exist even when the
> protocol is not defining them.
>
> --
> Nilay
>
> _______________________________________________
> 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