RFR (trivial): 8219888: aarch64 : add CPU detection code for HiSilicon TSV110

[Yangfei (Felix)]

Hi,

    Now it looks reasonable for Cavium and Broadcom sharing the same if-clause.  I was thinking they are totally different CPUs.  
    I can propose a v2 patch creating another if-clause for the HiSilicon one.  

Thanks,
Felix

> 
> Hi Felix,
> 
> I think I agree with your 2nd approach - a separate if-clause for each model. Even
> though a little code may be duplicated, it allows each vendor to make changes
> independently.
> 
> The Cavium/Broadcom block is a special case because they are essentially the
> same chip. The "Broadcom" versions were early manufacturing releases that are
> no longer relevant, so we might want to simplify this in future.
> 
>  - Derek
>  Cavium Inc. Oops, I mean Marvell Semiconductor Inc.
> 
> > -----Original Message-----
> > From: aarch64-port-dev <aarch64-port-dev-bounces at openjdk.java.net> On
> > Behalf Of Yangfei (Felix)
> > Sent: Thursday, February 28, 2019 10:08 PM
> > To: Patrick Zhang OS <patrick at os.amperecomputing.com>; hotspot-
> > runtime-dev at openjdk.java.net
> > Cc: aarch64-port-dev <aarch64-port-dev at openjdk.java.net>
> > Subject: [EXT] Re: [aarch64-port-dev ] RFR (trivial): 8219888: aarch64 : add
> > CPU detection code for HiSilicon TSV110
> >
> > External Email
> >
> > ----------------------------------------------------------------------
> > I am afraid we are duplicating code having one if-clause for each CPU.
> > Here, we have three different CPUs sharing this if-cause: CAVIUM,
> > BROADCOM and HISILICON.
> > It’s OK to place all vendor specific description together at the top, but I am
> > not clear about the model of BRAODCOM.
> > So I am going this way: separate description for each model.
> >
> >
> >
> > The top comment “says” this if-clause was for ThunderX2, which looks a little
> > strange.
> >
> > I would suggest either we place all vendor specific descriptions together at
> > the top, or have a new if-clause for each (much cleaner).
> >
> >
> >   // ThunderX2
> >   if ((_cpu == CPU_CAVIUM && (_model == 0xAF)) ||
> >       (_cpu == CPU_BROADCOM && (_model == 0x516)) ||
> >   // HiSilicon TSV110
> >       (_cpu == CPU_HISILICON && (_model == 0xd01))) {
> >     if (FLAG_IS_DEFAULT(AvoidUnalignedAccesses)) {
> >       FLAG_SET_DEFAULT(AvoidUnalignedAccesses, true);
> >     }
> >     if (FLAG_IS_DEFAULT(UseSIMDForMemoryOps)) {
> >       FLAG_SET_DEFAULT(UseSIMDForMemoryOps, true);
> >     }
> >   }
> >
> >
> >
> > Regards
> >
> > Patrick
> >
> >
> >
> > -----Original Message-----
> >
> > From: aarch64-port-dev <aarch64-port-dev-bounces at openjdk.java.net> On
> > Behalf Of Yangfei (Felix)
> >
> > Sent: Friday, March 1, 2019 9:23 AM
> >
> > To: hotspot-runtime-dev at openjdk.java.net
> >
> > Cc: aarch64-port-dev <aarch64-port-dev at openjdk.java.net>
> >
> > Subject: [aarch64-port-dev ] RFR (trivial): 8219888: aarch64 : add CPU
> > detection code for HiSilicon TSV110
> >
> >
> >
> > Hi,
> >
> >
> >
> >     Please review this trivial patch adding support for HiSilicon TSV110.
> >
> >     Bug: https://bugs.openjdk.java.net/browse/JDK-8219888
> >
> >     Webrev: http://cr.openjdk.java.net/~fyang/8219888/webrev.00/
> >
> >
> >
> >     Tested on Huawei Kunpeng 920 server platform.  OK to push?
> >
> >
> >
> > Thanks,
> >
> > Felix