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

[Derek White]

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