RFR(XS): 8238596: AVX enabled by default for Skylake even when unsupported

[David Buck]

Hi!

Yes Vladimir, I agree that your refactoring is easier to read. I would 
like to continue the code review using this new version.

updated webrev: http://cr.openjdk.java.net/~dbuck/8238596.1/

I have manually confirmed that the fix still works with VirtualBox. I 
have also rerun all of tier 1 and 2 builds / regression tests as well.

Aleksey, would you mind taking a quick look at the new version?

Cheers,
-Buck

On 2020/02/07 18:16, Vladimir Ivanov wrote:
> Hi David,
> 
>> webrev: http://cr.openjdk.java.net/~dbuck/8238596.0/
> 
> Good catch! Overall, looks good.
> 
> What do you think about the following variant?
> 
> 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
> @@ -672,11 +672,14 @@
>       }
>     }
>     if (FLAG_IS_DEFAULT(UseAVX)) {
> -    FLAG_SET_DEFAULT(UseAVX, use_avx_limit);
> -    if (is_intel_family_core() && _model == CPU_MODEL_SKYLAKE && 
> _stepping < 5) {
> -      FLAG_SET_DEFAULT(UseAVX, 2);  //Set UseAVX=2 for Skylake
> +    // Don't use AVX-512 on older Skylakes unless explicitly requested.
> +    if (use_avx_limit > 2 && is_intel_skylake() && _stepping < 5) {
> +      FLAG_SET_DEFAULT(UseAVX, 2);
> +    } else {
> +      FLAG_SET_DEFAULT(UseAVX, use_avx_limit);
>       }
> -  } else if (UseAVX > use_avx_limit) {
> +  }
> +  if (UseAVX > use_avx_limit) {
>       warning("UseAVX=%d is not supported on this CPU, setting it to 
> UseAVX=%d", (int) UseAVX, use_avx_limit);
>       FLAG_SET_DEFAULT(UseAVX, use_avx_limit);
>     } else if (UseAVX < 0) {
> 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
> @@ -868,6 +868,9 @@
>     static bool is_intel_family_core() { return is_intel() &&
>                                          extended_cpu_family() == 
> CPU_FAMILY_INTEL_CORE; }
> 
> +  static bool is_intel_skylake() { return is_intel_family_core() &&
> +                                          extended_cpu_model() == 
> CPU_MODEL_SKYLAKE; }
> +
>     static bool is_intel_tsc_synched_at_init()  {
>       if (is_intel_family_core()) {
>         uint32_t ext_model = extended_cpu_model();
> 
> 
> Best regards,
> Vladimir Ivanov