8292878: x86: Make scratch register usage explicit in assembler code

[iwanowww]

Improve MacroAssembler API and make scratch register usages explicit at use sites.

Different parts of JVM have different assumptions about scratch registers and
implicit assumptions in MacroAssembler makes it harder to reason about the
correctness of generated code.

Most of scratch register usages relate to AddressLiteral on x86_64. In such case the argument is turned into a default one (with noreg as a default) and implementation asserts that scratch register should be provided when the address is not guaranteed to be always reachable.

Otherwise, scratch register argument is required to be explicitly provided.

The only case left (mostly) intact is call(AddressLiteral), but I switched it from rscratch (r10) to rax.

Testing:

• hs-tier1 - hs-tier4 (w/ -XX:+-ForceUnreachable)
• x86_32 build

---

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-8292878: x86: Make scratch register usage explicit in assembler code

Reviewers

• Vladimir Kozlov (@vnkozlov - Reviewer) ⚠️ Review applies to ed4dc939
• Aleksey Shipilev (@shipilev - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 10003

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

Using diff file

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

[bridgekeeper]

👋 Welcome back vlivanov! 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.

[openjdk]

@iwanowww The following label will be automatically applied to this pull request:

• hotspot

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 03: Full - Incremental (f920a0fe)
• 02: Full - Incremental (ed4dc939)
• 01: Full - Incremental (2b0565ef)
• 00: Full (f2911037)

[vnkozlov]

I hope you did not do this all by hand ;)

Good.

[openjdk]

@iwanowww 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:

8292878: x86: Make scratch register usage explicit in assembler code

Reviewed-by: kvn, shade

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 68 new commits pushed to the master branch:

• f5ebbf3: 8292657: Calling GetLocalXXX from virtual thread with thread parameter set to NULL returns carrier locals
• 550e5d7: 4958071: (spec) confusing exception list for javax.crypto.Cipher.init(...) for
• d3d2e66: 8293009: Remove unused field 'millisPerHour' in DateFormatSymbols
• f3ebb42: 8293036: x86_64: Move Continuation-related stub methods to MacroAssembler
• 1576f99: 8288759: GCC 12 fails to compile signature.cpp due to -Wstringop-overread
• f285cea: 8293037: Remove DebuggerBase.writeBytes() and related code from SA
• 343333a: 8293004: (fs) FileChannel.transferXXX use of copy_file_range needs fallback handling for ENOSYS
• b3450e9: 8292850: Unused field 'expiredTimersKey' in javax.swing.TimerQueue
• 6b7d4b1: 8293028: Remove GenCollectedHeap::handle_failed_promotion
• 21a7369: 8292575: riscv: Represent Registers as values
• ... and 58 more: https://git.openjdk.org/jdk/compare/fe0544f8a7db5bf0e93313e16b54b31bcdff946a...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.

[iwanowww]

Accidentally, introduced a bug by turning some AddressLiterals into rvals.

Reverted relevant chanages, plus made the scratch register argument required for cmpptr(Address,AddressLiteral,Register).

Improved Assembler::reachable/Assembler::always_reachable to properly handle lvals.

Also, applied a minor refactoring around CompressedOops::ptrs_base_addr().

[vnkozlov]

Still good.

[iwanowww]

Thanks for the review, Vladimir.

@shipilev Aleksey, do you plan to look into x86_32-specific changes? Otherwise, I'll integrate the patch.

[shipilev]

@shipilev Aleksey, do you plan to look into x86_32-specific changes? Otherwise, I'll integrate the patch.

Missed this comment, sorry. Are you looking for a similar change on x86_32-specific code? I believe x86_32 code does not use scratch registers (they are #define-d to noreg), so there seem to be nothing to do. I see GHA infra barfed up on x86_32 tests, I'll run some tests locally.

[iwanowww]

Thanks. Just wanted to hear from you since it affects x86_32 in a non-trivial way.

[shipilev]

I think we are fine. tier1 and tier2 pass on x86_32.

[iwanowww]

Thanks, Aleksey.

/integrate

[openjdk]

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

• f5ebbf3: 8292657: Calling GetLocalXXX from virtual thread with thread parameter set to NULL returns carrier locals
• 550e5d7: 4958071: (spec) confusing exception list for javax.crypto.Cipher.init(...) for
• d3d2e66: 8293009: Remove unused field 'millisPerHour' in DateFormatSymbols
• f3ebb42: 8293036: x86_64: Move Continuation-related stub methods to MacroAssembler
• 1576f99: 8288759: GCC 12 fails to compile signature.cpp due to -Wstringop-overread
• f285cea: 8293037: Remove DebuggerBase.writeBytes() and related code from SA
• 343333a: 8293004: (fs) FileChannel.transferXXX use of copy_file_range needs fallback handling for ENOSYS
• b3450e9: 8292850: Unused field 'expiredTimersKey' in javax.swing.TimerQueue
• 6b7d4b1: 8293028: Remove GenCollectedHeap::handle_failed_promotion
• 21a7369: 8292575: riscv: Represent Registers as values
• ... and 58 more: https://git.openjdk.org/jdk/compare/fe0544f8a7db5bf0e93313e16b54b31bcdff946a...master

Your commit was automatically rebased without conflicts.

[openjdk]

@iwanowww Pushed as commit 6e24827.

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