8351016: RA support for EVEX to REX/REX2 demotion to optimize NDD instructions

[jatin-bhateja]

Currently, while choosing the colour (register) for a definition live range during the select phase of register allocation, we pick the first available colour that does not match with already allocated neighboring live ranges.

With Intel APX NDD ISA extension, several existing two-address arithmetic instructions can now have an explicit non-destructive destination operand; this, in general, saves additional spills for two-address instructions where the destination is also the first source operand, and where the source live range surpasses the current instruction.

All NDD instructions mandate extended EVEX encoding with a bulky 4-byte prefix, JDK-8351994 added logic for NDD to REX/REX2 demotion in the assembler layer, but due to the existing first color selection register allocation policy, the demotions are rare. This patch biases the allocation of NDD definition to the first source operand or the second source operand for the commutative class of operations.

Biasing is a compile-time hint to the allocator and is different from live range coalescing (aggressive/conservative), which merges the two live ranges using the union find algorithm. Given that REX encoding needs a 1-byte prefix and REX2 encoding needs a 2-byte prefix, domotion saves considerable JIT code size.

The patch shows around 5-20% improvement in code size by facilitating NDD demotion.

For the following micro, the method JIT code size reduced from 136 to 120 bytes, which is around a 13% reduction in code size footprint.

Micro:-

Baseline :-

With opt:-

Thorough validations are underway using the latest Intel Software Development Emulator version 9.58.

Kindly review and share your feedback.

Best Regards,
Jatin

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8351016: RA support for EVEX to REX/REX2 demotion to optimize NDD instructions (Sub-task - P4)

Reviewers

• Sandhya Viswanathan (@sviswa7 - Reviewer)
• Quan Anh Mai (@merykitty - Committer) Review applies to e2f95f31
• Daniel Lundén (@dlunde - Committer)
• Vladimir Ivanov (@iwanowww - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/26283/head:pull/26283
$ git checkout pull/26283

Update a local copy of the PR:
$ git checkout pull/26283
$ git pull https://git.openjdk.org/jdk.git pull/26283/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 26283

View PR using the GUI difftool:
$ git pr show -t 26283

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/26283.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back jbhateja! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[jatin-bhateja]

/keepalive

[openjdk]

@jatin-bhateja This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8351016: RA support for EVEX to REX/REX2 demotion to optimize NDD instructions

Reviewed-by: sviswanathan, dlunden, vlivanov, qamai

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 35 new commits pushed to the master branch:

• 3fd551f: 8371769: TestMemoryInvisibleParent.java fails with java.nio.file.AccessDeniedException
• 282f339: 8369432: Add Support for JDBC 4.5 MR
• 92e1357: 8371802: Do not let QUIC connection to idle terminate when HTTP/3 is configured with a higher idle timeout
• ... and 32 more: https://git.openjdk.org/jdk/compare/1ce2a44e9f4fa9d558602dbd0489fefb0c9563ef...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[jatin-bhateja]

/label add hotspot-compiler-dev

[openjdk]

@jatin-bhateja The pull request is being re-evaluated and the inactivity timeout has been reset.

[openjdk]

@jatin-bhateja
The hotspot-compiler label was successfully added.

[merykitty]

This may also be applicable to non-APX instructions. For example, in the case of casting long to int, if the destination and the source are the same, then we do not need to emit any code. As a result, do you think it is better to mark operands in the ad file to preferably have the same register as the result?

[jatin-bhateja]

This may also be applicable to non-APX instructions. For example, in the case of casting long to int, if the destination and the source are the same, then we do not need to emit any code. As a result, do you think it is better to mark operands in the ad file to preferably have the same register as the result?

Yes, for now, I limited this to APX, but biasing the allocation of destination to non-interfering use(src) will enable instruction elision during assembling. Currently, while assigning a color(reg) to a live range, the allocator picks the first free aligned register.

do you think it is better to mark operands in the ad file to preferably have the same register as the result?

There are existing DF attributions like USE_DEF which can be used to add such a constraint, but for APX NDD, we do not wish to up-front constrain the source to be the same as the destination, as it defeats the purpose, and the allocator may end up emitting a copy before the NDD instruction to honour this constraint. The idea here is to only bias color selection for non-interfering live ranges to facilitate EEVEX to REX/REX2 demotions.

[mlbridge]

Webrevs

• 22: Full (93577b83)
• 21: Full - Incremental (2fe2ab2e)
• 20: Full - Incremental (d596c232)
• 19: Full - Incremental (912d109a)
• 18: Full - Incremental (57a9e4bf)
• 17: Full - Incremental (bb41ff78)
• 16: Full - Incremental (038a292e)
• 15: Full - Incremental (151b51af)
• 14: Full - Incremental (72a5f876)
• 13: Full - Incremental (f0513b87)
• 12: Full - Incremental (ee8b0368)
• 11: Full - Incremental (cccef216)
• 10: Full - Incremental (ef51f875)
• 09: Full - Incremental (6c359e87)
• 08: Full (038eebdb)
• 07: Full - Incremental (e2f95f31)
• 06: Full - Incremental (31079431)
• 05: Full (e7de7ae6)
• 04: Full - Incremental (1ab0ac92)
• 03: Full (5ae56d8d)
• 02: Full - Incremental (3ebe52fa)
• 01: Full (cd13fe60)
• 00: Full (7ffb2102)

[sviswa7]

@jatin-bhateja Thanks for looking into this.

[openjdk]

@jatin-bhateja To determine the appropriate audience for reviewing this pull request, one or more labels corresponding to different subsystems will normally be applied automatically. However, no automatic labelling rule matches the changes in this pull request. In order to have an "RFR" email sent to the correct mailing list, you will need to add one or more applicable labels manually using the /label pull request command.

Applicable Labels

• build
• client
• compiler
• core-libs
• graal
• hotspot
• hotspot-compiler
• hotspot-gc
• hotspot-jfr
• hotspot-runtime
• i18n
• ide-support
• javadoc
• jdk
• jmx
• net
• nio
• security
• serviceability
• shenandoah

[sviswa7]

Looks good to me.

[merykitty]

I can't approve this approach. I think blindly biasing the color of an operation to that of its input is too optimistic and will lead to numerous false-positive cases. It is better to have a more fine-grained selection using the script in the ad file. For example:

instruct addI_rReg_ndd(rRegI dst, rRegI src1, rRegI src2, rFlagsReg cr)
%{
  predicate(UseAPX);
  match(Set dst (AddI src1 src2));
  effect(KILL cr);
  bias(src1);
  flag(PD::Flag_sets_overflow_flag, PD::Flag_sets_sign_flag, PD::Flag_sets_zero_flag, PD::Flag_sets_carry_flag, PD::Flag_sets_parity_flag);

  format %{ "eaddl    $dst, $src1, $src2\t# int ndd" %}
  ins_encode %{
    __ eaddl($dst$$Register, $src1$$Register, $src2$$Register, false);
  %}
  ins_pipe(ialu_reg_reg);
%}

[openjdk]

@jatin-bhateja Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. See OpenJDK Developers’ Guide for more information.

[jatin-bhateja]

Hi @iwanowww , Please let me know if this is good to land now.

[iwanowww]

Thanks, Jatin. Overall, looks good.

[openjdk]

@jatin-bhateja Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. See OpenJDK Developers’ Guide for more information.

[iwanowww]

Thanks for clarifications, Jatin. Looks good.

[jatin-bhateja]

Thanks @iwanowww , for your comments and review approval.

Hi @sviswa7 , @dlunde can you check and re-approve this version.

Best Regards

[dlunde]

Rerunning tests and will re-approve when finished. Latest changes look good, here are a few nits (only comment and style changes): e33416a

Also, thanks for your patience!

[jatin-bhateja]

Hi @dlunde , waiting for your test clearance. thanks!

[dlunde]

Tests look good!

[jatin-bhateja]

Hi @sviswa7. @iwanowww , can you kindly re-approve this.

[jatin-bhateja]

Thanks @iwanowww , @dean-long, @dlunde , @merykitty and @sviswa7 for your reviews and approval

[jatin-bhateja]

/integrate

[openjdk]

Going to push as commit e0311ec.
Since your change was applied there have been 36 commits pushed to the master branch:

• c7a489d: 8372534: Update Libpng to 1.6.51
• 3fd551f: 8371769: TestMemoryInvisibleParent.java fails with java.nio.file.AccessDeniedException
• 282f339: 8369432: Add Support for JDBC 4.5 MR
• ... and 33 more: https://git.openjdk.org/jdk/compare/1ce2a44e9f4fa9d558602dbd0489fefb0c9563ef...master

Your commit was automatically rebased without conflicts.

[openjdk]

@jatin-bhateja Pushed as commit e0311ec.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.