[PATCH v3] JDK-8222090: Add Hygon Dhyana support
David Holmes
david.holmes at oracle.com
Thu Apr 11 02:47:10 UTC 2019
On 11/04/2019 12:48 pm, Vladimir Kozlov wrote:
> On 4/10/19 6:14 PM, David Holmes wrote:
>> Hi Vladimir,
>>
>> Thanks for taking look at this.
>>
>> On 11/04/2019 2:41 am, Vladimir Kozlov wrote:
>>> Instead of repeated is_amd() || is_hygon() you can use
>>> is_amd_or_hygon() to be more clean.
>>
>> That doesn't really scale if we get more "cousins" joining the AMD
>> family. I prefer to see the explicit disjunction form.
>
> is_amd_family() ? If it was 2 or 3 cases I would agree with you. But you
> have 10 checks!
is_amd_family() seems reasonable and scalable - thanks.
>
>>
>>> Next check should be explicit for CPU:
>>>
>>> + // Some defaults for AMD family 17h || Hygon family 18h
>>> + if ( cpu_family() == 0x17 || cpu_family() == 0x18 ) {
>>>
>>> ---
>>>
>>> + // Some defaults for AMD family 17h || Hygon family 18h
>>> + if ( (cpu_family() == 0x17) && is_amd() || (cpu_family() ==
>>> 0x18) && is_hygon()) {
>>
>> I'm wondering if that should just be an assertion? I would not expect
>> it to be possible to have a family 0x17 that is not AMD and likewise
>> for family 0x18 and Hygon. But then I don't know how these family ids
>> get issued. ??
>
> It is not clear what agreement AMD has with these companies. Any company
> (as AMD and Intel) may have own set of cpu_family ids and we can't
> assume that ids will not be reused. They have only 8 bits for ID.
Yes I dug a bit deeper and found for example that both Intel and AMD use
0xF - so it need not be unique and so we should check family id and
vendor id. (Might need to re-examine existing code that only uses family
id!)
Thanks,
David
> Thanks,
> Vladimir
>
>>
>> Thanks,
>> David
>>
>>> Thanks,
>>> Vladimir
>>>
>>> On 4/10/19 12:33 AM, David Holmes wrote:
>>>> Hi Fanjinke,
>>>>
>>>> This looks fine to me. I've updated the webrev at:
>>>>
>>>> http://cr.openjdk.java.net/~dholmes/8222090/webrev.v3/
>>>>
>>>> We need a second reviewer before I push it.
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>> On 9/04/2019 5:22 pm, Jinke Fan wrote:
>>>>> diff --git a/src/hotspot/cpu/x86/assembler_x86.cpp
>>>>> b/src/hotspot/cpu/x86/assembler_x86.cpp
>>>>> --- a/src/hotspot/cpu/x86/assembler_x86.cpp
>>>>> +++ b/src/hotspot/cpu/x86/assembler_x86.cpp
>>>>> @@ -3099,7 +3099,7 @@
>>>>> }
>>>>> return;
>>>>> }
>>>>> - if (UseAddressNop && VM_Version::is_amd()) {
>>>>> + if (UseAddressNop && (VM_Version::is_amd() ||
>>>>> VM_Version::is_hygon())) {
>>>>> //
>>>>> // Using multi-bytes nops "0x0F 0x1F [address]" for AMD.
>>>>> // 1: 0x90
>>>>> diff --git a/src/hotspot/cpu/x86/vm_version_ext_x86.cpp
>>>>> b/src/hotspot/cpu/x86/vm_version_ext_x86.cpp
>>>>> --- a/src/hotspot/cpu/x86/vm_version_ext_x86.cpp
>>>>> +++ b/src/hotspot/cpu/x86/vm_version_ext_x86.cpp
>>>>> @@ -340,6 +340,10 @@
>>>>> return !is_amd_Barcelona();
>>>>> }
>>>>>
>>>>> + if (is_hygon()) {
>>>>> + return true;
>>>>> + }
>>>>> +
>>>>> return false;
>>>>> }
>>>>>
>>>>> @@ -407,6 +411,10 @@
>>>>> }
>>>>> return _family_id_intel[cpu_family_id];
>>>>> }
>>>>> + if (is_hygon()) {
>>>>> + return "Dhyana";
>>>>> + }
>>>>> +
>>>>> return "Unknown x86";
>>>>> }
>>>>>
>>>>> @@ -423,6 +431,9 @@
>>>>> } else if (is_amd()) {
>>>>> cpu_type = "AMD";
>>>>> x64 = cpu_is_em64t() ? " AMD64" : "";
>>>>> + } else if (is_hygon()) {
>>>>> + cpu_type = "Hygon";
>>>>> + x64 = cpu_is_em64t() ? " AMD64" : "";
>>>>> } else {
>>>>> cpu_type = "Unknown x86";
>>>>> x64 = cpu_is_em64t() ? " x86_64" : "";
>>>>> diff --git a/src/hotspot/cpu/x86/vm_version_x86.cpp
>>>>> b/src/hotspot/cpu/x86/vm_version_x86.cpp
>>>>> --- a/src/hotspot/cpu/x86/vm_version_x86.cpp
>>>>> +++ b/src/hotspot/cpu/x86/vm_version_x86.cpp
>>>>> @@ -1165,7 +1165,7 @@
>>>>> }
>>>>> }
>>>>>
>>>>> - if( is_amd() ) { // AMD cpus specific settings
>>>>> + if( is_amd() || is_hygon() ) { // AMD cpus specific settings
>>>>> if( supports_sse2() && FLAG_IS_DEFAULT(UseAddressNop) ) {
>>>>> // Use it on new AMD cpus starting from Opteron.
>>>>> UseAddressNop = true;
>>>>> @@ -1239,8 +1239,8 @@
>>>>> }
>>>>> #endif // COMPILER2
>>>>>
>>>>> - // Some defaults for AMD family 17h
>>>>> - if ( cpu_family() == 0x17 ) {
>>>>> + // Some defaults for AMD family 17h || Hygon family 18h
>>>>> + if ( cpu_family() == 0x17 || cpu_family() == 0x18 ) {
>>>>> // On family 17h processors use XMM and UnalignedLoadStores
>>>>> for Array Copy
>>>>> if (supports_sse2() && FLAG_IS_DEFAULT(UseXMMForArrayCopy)) {
>>>>> FLAG_SET_DEFAULT(UseXMMForArrayCopy, true);
>>>>> diff --git a/src/hotspot/cpu/x86/vm_version_x86.hpp
>>>>> b/src/hotspot/cpu/x86/vm_version_x86.hpp
>>>>> --- a/src/hotspot/cpu/x86/vm_version_x86.hpp
>>>>> +++ b/src/hotspot/cpu/x86/vm_version_x86.hpp
>>>>> @@ -495,13 +495,13 @@
>>>>> result |= CPU_CX8;
>>>>> if (_cpuid_info.std_cpuid1_edx.bits.cmov != 0)
>>>>> result |= CPU_CMOV;
>>>>> - if (_cpuid_info.std_cpuid1_edx.bits.fxsr != 0 || (is_amd() &&
>>>>> + if (_cpuid_info.std_cpuid1_edx.bits.fxsr != 0 || ((is_amd() ||
>>>>> is_hygon()) &&
>>>>> _cpuid_info.ext_cpuid1_edx.bits.fxsr != 0))
>>>>> result |= CPU_FXSR;
>>>>> // HT flag is set for multi-core processors also.
>>>>> if (threads_per_core() > 1)
>>>>> result |= CPU_HT;
>>>>> - if (_cpuid_info.std_cpuid1_edx.bits.mmx != 0 || (is_amd() &&
>>>>> + if (_cpuid_info.std_cpuid1_edx.bits.mmx != 0 || ((is_amd() ||
>>>>> is_hygon()) &&
>>>>> _cpuid_info.ext_cpuid1_edx.bits.mmx != 0))
>>>>> result |= CPU_MMX;
>>>>> if (_cpuid_info.std_cpuid1_edx.bits.sse != 0)
>>>>> @@ -576,8 +576,8 @@
>>>>> if (_cpuid_info.std_cpuid1_ecx.bits.fma != 0)
>>>>> result |= CPU_FMA;
>>>>>
>>>>> - // AMD features.
>>>>> - if (is_amd()) {
>>>>> + // AMD|Hygon features.
>>>>> + if (is_amd() || is_hygon()) {
>>>>> if ((_cpuid_info.ext_cpuid1_edx.bits.tdnow != 0) ||
>>>>> (_cpuid_info.ext_cpuid1_ecx.bits.prefetchw != 0))
>>>>> result |= CPU_3DNOW_PREFETCH;
>>>>> @@ -711,6 +711,7 @@
>>>>> static int cpu_family() { return _cpu;}
>>>>> static bool is_P6() { return cpu_family() >= 6; }
>>>>> static bool is_amd() { assert_is_initialized();
>>>>> return _cpuid_info.std_vendor_name_0 == 0x68747541; } // 'htuA'
>>>>> + static bool is_hygon() { assert_is_initialized();
>>>>> return _cpuid_info.std_vendor_name_0 == 0x6F677948; } // 'ogyH'
>>>>> static bool is_intel() { assert_is_initialized();
>>>>> return _cpuid_info.std_vendor_name_0 == 0x756e6547; } // 'uneG'
>>>>> static bool is_zx() { assert_is_initialized();
>>>>> return (_cpuid_info.std_vendor_name_0 == 0x746e6543) ||
>>>>> (_cpuid_info.std_vendor_name_0 == 0x68532020); } // 'tneC'||'hS '
>>>>> static bool is_atom_family() { return ((cpu_family() ==
>>>>> 0x06) && ((extended_cpu_model() == 0x36) || (extended_cpu_model()
>>>>> == 0x37) || (extended_cpu_model() == 0x4D))); } //Silvermont and
>>>>> Centerton
>>>>> @@ -734,7 +735,7 @@
>>>>> if (!supports_topology || result == 0) {
>>>>> result = (_cpuid_info.dcp_cpuid4_eax.bits.cores_per_cpu +
>>>>> 1);
>>>>> }
>>>>> - } else if (is_amd()) {
>>>>> + } else if (is_amd() || is_hygon()) {
>>>>> result = (_cpuid_info.ext_cpuid8_ecx.bits.cores_per_cpu + 1);
>>>>> } else if (is_zx()) {
>>>>> bool supports_topology = supports_processor_topology();
>>>>> @@ -770,7 +771,7 @@
>>>>> intx result = 0;
>>>>> if (is_intel()) {
>>>>> result = (_cpuid_info.dcp_cpuid4_ebx.bits.L1_line_size + 1);
>>>>> - } else if (is_amd()) {
>>>>> + } else if (is_amd() || is_hygon()) {
>>>>> result = _cpuid_info.ext_cpuid5_ecx.bits.L1_line_size;
>>>>> } else if (is_zx()) {
>>>>> result = (_cpuid_info.dcp_cpuid4_ebx.bits.L1_line_size + 1);
>>>>> @@ -857,7 +858,7 @@
>>>>>
>>>>> // AMD features
>>>>> static bool supports_3dnow_prefetch() { return (_features &
>>>>> CPU_3DNOW_PREFETCH) != 0; }
>>>>> - static bool supports_mmx_ext() { return is_amd() &&
>>>>> _cpuid_info.ext_cpuid1_edx.bits.mmx_amd != 0; }
>>>>> + static bool supports_mmx_ext() { return (is_amd()||is_hygon())
>>>>> && _cpuid_info.ext_cpuid1_edx.bits.mmx_amd != 0; }
>>>>> static bool supports_lzcnt() { return (_features &
>>>>> CPU_LZCNT) != 0; }
>>>>> static bool supports_sse4a() { return (_features &
>>>>> CPU_SSE4A) != 0; }
>>>>>
>>>>> @@ -870,7 +871,7 @@
>>>>> }
>>>>> static bool supports_tscinv() {
>>>>> return supports_tscinv_bit() &&
>>>>> - ( (is_amd() && !is_amd_Barcelona()) ||
>>>>> + ( ((is_amd()||is_hygon()) && !is_amd_Barcelona()) ||
>>>>> is_intel_tsc_synched_at_init() );
>>>>> }
>>>>>
>>>>> @@ -896,7 +897,7 @@
>>>>> // Core - 256 / prefetchnta
>>>>> // It will be used only when AllocatePrefetchStyle > 0
>>>>>
>>>>> - if (is_amd()) { // AMD
>>>>> + if (is_amd() || is_hygon()) { // AMD
>>>>> if (supports_sse2()) {
>>>>> return 256; // Opteron
>>>>> } else {
More information about the hotspot-dev
mailing list