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

Reply via email to