RFR: 8324734: Relax too-strict assert(VM_Version::supports_evex()) in Assembler::locate_operand() [v2]

Aleksey Shipilev shade at openjdk.org
Mon Jan 29 18:38:37 UTC 2024


On Mon, 29 Jan 2024 09:30:02 GMT, Roman Kennke <rkennke at openjdk.org> wrote:

>> Details see bug report. The gist is that HotSpot downgrades to UseAVX=2 on some processors, and reports supports_evex() == false, but the instruction decoder can still encounter EVEX instructions when (e.g.) hitting a SIGBUS in memset() - which does have EVEX instructions.
>> 
>> Testing:
>>  - [x] runtime/Unsafe/InternalErrorTest.java
>>  - [x] tier1
>
> Roman Kennke has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Distinguish between CPU and HotSpot features for supports_evex()

This looks fine, with nits.

src/hotspot/cpu/x86/vm_version_x86.cpp line 812:

> 810: 
> 811:   if (cpu_family() > 4) { // it supports CPUID
> 812:     _features = feature_flags(); // It can be changed by VM flags

Suggestion:

    _features = feature_flags(); // These can be changed by VM settings

src/hotspot/cpu/x86/vm_version_x86.hpp line 643:

> 641: 
> 642:   //
> 643:   // Feature identification which can be affected by VM flags

Suggestion:

  // Feature identification which can be affected by VM settings

src/hotspot/cpu/x86/vm_version_x86.hpp line 709:

> 707:   // Feature identification not affected by VM flags
> 708:   //
> 709:   static bool cpu_supports_evex()         { return (_cpu_features & CPU_AVX512F) != 0; }

Indenting is a bit off here. I think the opening brace should be aligned with the others on the top.

src/hotspot/share/runtime/abstract_vm_version.hpp line 57:

> 55:   static const char*  _s_internal_vm_info_string;
> 56: 
> 57:   // CPU feature flags which can be restricted by VM flags.

Suggestion:

  // CPU feature flags, can be affected by VM settings.

src/hotspot/share/runtime/abstract_vm_version.hpp line 61:

> 59:   static const char* _features_string;
> 60: 
> 61:   // CPU feature flags not affected by VM flags.

Suggestion:

  // Original CPU feature flags, not affected by VM settings.

-------------

Marked as reviewed by shade (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/17590#pullrequestreview-1849488534
PR Review Comment: https://git.openjdk.org/jdk/pull/17590#discussion_r1470022549
PR Review Comment: https://git.openjdk.org/jdk/pull/17590#discussion_r1470032016
PR Review Comment: https://git.openjdk.org/jdk/pull/17590#discussion_r1470021986
PR Review Comment: https://git.openjdk.org/jdk/pull/17590#discussion_r1470032487
PR Review Comment: https://git.openjdk.org/jdk/pull/17590#discussion_r1470033274


More information about the hotspot-compiler-dev mailing list