RFR: 8329031: CPUID feature detection for Advanced Performance Extensions (Intel® APX) [v2]

[Jatin Bhateja]

On Tue, 4 Jun 2024 23:58:50 GMT, Sandhya Viswanathan <sviswanathan at openjdk.org> wrote:

>> Jatin Bhateja has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Review comments resolutions.
>
> src/hotspot/cpu/x86/vm_version_x86.cpp line 113:
> 
>> 111:   VM_Version_StubGenerator(CodeBuffer *c) : StubCodeGenerator(c) {}
>> 112: 
>> 113:   address clear_apx_test_state() {
> 
> Why do we need to clear_apx_test_state? r16 onwards are not callee saved. And checking r15 save/restore is not needed so we could remove r15 changes altogether.

Yes, EGPRs are call clobbered registers, but here we are trying to ascertain if their values are preserved across signal handling. Explicit clearing of r16 and r31 during signal handling guarantees that preserved register values post signal handling were re-instantiated by operating system and not because they were not modified externally.

> src/hotspot/cpu/x86/vm_version_x86.cpp line 447:
> 
>> 445:     /* FIXME: Uncomment after integration of JDK-8328998
>> 446:     __ mov64(rax, VM_Version::egpr_test_value());
>> 447:     __ cmpq(rax, r15);
> 
> Likewise r15 validation can be removed.

r15 validation showed contrasting results in comparison to r16 currently, But its fair enough to remove it. DONE

> src/hotspot/cpu/x86/vm_version_x86.cpp line 456:
> 
>> 454:     // Generate SEGV to signal unsuccessful save/restore.
>> 455:     __ bind(apx_save_restore_error);
>> 456:     __ lea(rax, ExternalAddress(VM_Version::_apx_state_restore_error_handler));
> 
> Generating an error message here won't be the right thing (especially since this is default by feature detection). It should only result in setting UseAPX feature to false.

DONE

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18562#discussion_r1630107399
PR Review Comment: https://git.openjdk.org/jdk/pull/18562#discussion_r1630107493
PR Review Comment: https://git.openjdk.org/jdk/pull/18562#discussion_r1630108400