RFR: 8329032: C2 compiler register allocation support for APX EGPRs [v3]

[Vladimir Kozlov]

On Mon, 17 Jun 2024 05:53:35 GMT, Jatin Bhateja <jbhateja at openjdk.org> wrote:

>> Intel® Advanced Performance Extensions (Intel® APX) adds 16 new 64 bit general purpose register also known as Extended General Purpose Registers in IA-32e 64 bit mode.
>> 
>> Summary of changes introduced along with this patch:-
>> 
>> 1. C2 compiler register allocation support.
>> 2. Architecture state save restoration while transitioning from C1/C2 JIT compiled code to runtime services.
>> 3. Support new PUSH2/POP2 instructions along with push-pop acceleration hints (PPX) to optimize register save/restore operation.
>> 4. Applicable extensions to native interface used by runtime for patching instruction.
>> 
>> We plan to address C1 register support in subsequent patch as there are hard upper bound allocation limits
>> (currently set to r11) imposed by existing implementation of linear scan algorithm after which it reserves
>> remaining register for special purpose.
>> 
>> Patch has been regressed over stand alone test points after prioritizing EGPR allocations over existing GPR register by manually modifying the register sequences in relevant allocation class.
>> 
>> We plan to do thorough validation using [Intel's SDE](https://www.intel.com/content/www/us/en/download/684897/intel-software-development-emulator.html) during course of time and release incremental patches for bug fixes
>> found during testing.
>> 
>> Kindly review and share your feedback.
>> 
>> Best Regards,
>> Jatin
>
> Jatin Bhateja has updated the pull request incrementally with one additional commit since the last revision:
> 
>   jvmci test failures fixes

I have comments.

src/hotspot/cpu/x86/nativeInst_x86.hpp line 469:

> 467: // mov rbx, [rip + offset]
> 468: //FIXME: Currently not being used in hotspot code base, extend it to support REX2 prefix.
> 469: class NativeLoadGot: public NativeInstruction {

Leftover from JAOTC implementation. There are few other methods we forgot to remove.
You don't need to fix it. Please, remove the comment.

src/hotspot/cpu/x86/nativeInst_x86.hpp line 574:

> 572: 
> 573: // far jump reg
> 574: //FIXME: Currently not being used in hotspot code base, extend it to support REX2 prefix.

JAOTC leftover.

src/hotspot/cpu/x86/nativeInst_x86.hpp line 629:

> 627: 
> 628: //FIXME: Register indirect jump interface, currently not being used in hotspot code base,
> 629: // extend it to support REX2 prefix if needed.

JAOTC leftover.

src/hotspot/cpu/x86/nativeInst_x86.hpp line 670:

> 668: }
> 669: 
> 670: //FIXME: Currently not being used in hotspot code base, extend it to support REX2 prefix.

Some very old code - never used.

src/hotspot/cpu/x86/sharedRuntime_x86_64.cpp line 226:

> 224:   __ subq(rsp, 16 * wordSize);
> 225: 
> 226:   __ movq(Address(rsp, 15 * wordSize), rax);

Consider moving saving register code into separate method. You have 3 places where you use it.

src/hotspot/cpu/x86/sharedRuntime_x86_64.cpp line 535:

> 533:   __ pop_FPU_state();
> 534: 
> 535:   __ movq(r15, Address(rsp, 0));

The same for restoring registers.

src/hotspot/cpu/x86/sharedRuntime_x86_64.cpp line 3375:

> 3373:   ResourceMark rm;
> 3374: 
> 3375:   CodeBuffer buffer(name, 1752, 512);

What cause the need to such increase of size?

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

> 420:     __ movl(Address(rsi,12), rdx);
> 421: 
> 422: #if !defined(PRODUCT) && defined(_LP64)

Why it is still under `!PRODUCT`?  You need to check if OS supports it in product VM too. Or I missing something?

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

PR Review: https://git.openjdk.org/jdk/pull/19042#pullrequestreview-2123322141
PR Review Comment: https://git.openjdk.org/jdk/pull/19042#discussion_r1643063458
PR Review Comment: https://git.openjdk.org/jdk/pull/19042#discussion_r1643072642
PR Review Comment: https://git.openjdk.org/jdk/pull/19042#discussion_r1643065954
PR Review Comment: https://git.openjdk.org/jdk/pull/19042#discussion_r1643079601
PR Review Comment: https://git.openjdk.org/jdk/pull/19042#discussion_r1643084935
PR Review Comment: https://git.openjdk.org/jdk/pull/19042#discussion_r1643085861
PR Review Comment: https://git.openjdk.org/jdk/pull/19042#discussion_r1643087605
PR Review Comment: https://git.openjdk.org/jdk/pull/19042#discussion_r1643089973