Am 12.12.2012 16:05, schrieb Eduardo Habkost: > On Wed, Dec 12, 2012 at 03:47:49PM +0100, Andreas Färber wrote: >> Am 12.12.2012 13:45, schrieb Eduardo Habkost: >>> On Mon, Dec 10, 2012 at 11:59:32PM +0100, Andreas Färber wrote: >>>> ehabkost: "When adding the Haswell CPU model, I intended to make it >>>> a superset of the features present on the SandyBridge model" >>>> >>>> Inherit from SandyBridge to keep only the delta for Haswell. >>> >>> Most CPUs have a superset of the features of their predecessors. Are you >>> simply using SandyBridge->Haswell as an example, or you think their >>> relationship is special somehow? >>> >>> I believe we don't want to make externally-visible class inheritance, >>> but probably just reuse constans or init functions internally. A Haswell >>> CPU is not a type of SandyBridge CPU, it just happens to contain a >>> superset of the features present in SandyBridge. >>> >>> I mean: Haswell also has a superset of features of 486, but we don't >>> want to make the hierarchy look like the following, do we? >> >> I don't see why we would want to use a #define-based inheritence as >> suggested for the PPRO when we have QOM. QOM inheritence reduces lines >> of code significantly compared to just taking the values from elsewhere. > > The reuse doesn't need to be #define-based (although maybe a > #define-based approach would work too), it could be function-call-based. > >> >> For the Haswell you said what I quoted, for the other models I said I >> need your or someone's help to verify whether a hierarchy such as below >> is semantically right or just a coincidence. I was at least considering >> an abstract intel-/amd-*-cpu to avoid repeating the three value >> assignments over and over. > > Creating X86IntelCPU and X86AMDCPU classes make sense to me, because > Haswell is a kind of Intel CPu. Making Haswell a subclass of 486 (like > below) wouldn't. > >> >> At this time I believe the parents of a type are not (yet) exposed via >> QMP, just the "type" string property. > > Even if they are not exposed externally, it's a confusing usage of > inheritance for me. I mean: a Haswell CPU is not a type of 486 CPU, it's > simply a different kind of CPU that happens to have a superset of the > 486 features.
I concur with your is-a argument. My patch took a pragmatic approach. I'd like to wait for some more review comments on how to share code between models then - I remember Paul, Anthony and Alex discussing the x86 models a while back on IRC, CC'ing. (Summary: reading CPU models from config files has been dropped, we only have built-in models left - now how to design name-class mappings below X86CPU) Andreas >>> - X86CPU >>> -> X86IntelCPU >>> -> 486 >>> -> pentium >>> -> pentium2 >>> -> pentium3 >>> -> Conroe >>> -> Penryn >>> -> Nehalem >>> -> Westmere >>> -> SandyBridge >>> -> Haswell -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg