8318959: C2: define MachNode::fill_new_machnode() statically

[robcasloz]

This changeset makes the implementation of MachNode::fill_new_machnode() static instead of ADLC-generated, for better maintainability. The original motivation to generate this function was to replace its logic with a ShouldNotCallThis() call for targets that do not define CISC spill or short branch instruction versions. However, most targets (including x64 and aarch64) feature at least one of these and hence do not make use of the ADLC specialization. For those that do, the savings (approximately 140 bytes on a x64 build of libjvm.so) are marginal and (arguably) not worth the additional code complexity.

See motivating discussion in https://mail.openjdk.org/pipermail/hotspot-compiler-dev/2023-October/thread.html#69348.

Testing

• tier1-3 (windows-x64, linux-x64, linux-aarch64, macosx-x64, macosx-aarch64; release and debug mode)

---

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-8318959: C2: define MachNode::fill_new_machnode() statically (Enhancement - P4)

Reviewers

• Vladimir Kozlov (@vnkozlov - Reviewer)
• Tobias Hartmann (@TobiHartmann - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 16444

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

Using diff file

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

Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back rcastanedalo! 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]

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

• hotspot-compiler

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

• 01: Full - Incremental (af289b04)
• 00: Full (276f39c4)

[vnkozlov]

Good.

[openjdk]

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

8318959: C2: define MachNode::fill_new_machnode() statically

Reviewed-by: kvn, thartmann

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

• ec79ab4: 8319268: Build failure with GCC8.3.1 after 8313643
• c788160: 8296240: Augment discussion of test tiers in doc/testing.md
• ffaecd4: 8315364: Assert thread state invariant for JFR stack trace capture
• 3b65b87: 8316028: Update FreeType to 2.13.2
• 9dc40ba: 8319195: Move most tier 1 vector API regression tests to tier 3
• f875163: 8318982: Improve Exceptions::special_exception
• 84f4f74: 8319213: Compatibility.java reads both stdout and stderr of JdkUtils
• 6ad093e: 8319265: TestLoadLibraryDeadlock.java fails on windows-x64 "Unable to load b.jar"
• e6f46a4: 8317374: Add Let's Encrypt ISRG Root X2
• e318cd2: 8316969: Improve CDS module graph support for --module option
• ... and 41 more: https://git.openjdk.org/jdk/compare/e05cafda78a37dbeb2df2edd791be19d22edaece...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.

[robcasloz]

Good.

Thanks for reviewing, Vladimir!

[TobiHartmann]

Looks good.

[robcasloz]

Looks good.

Thanks, Tobias!

[robcasloz]

/integrate

[openjdk]

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

• c146685: 8319165: hsdis binutils: warns on empty string as option string
• 01c0d5d: 8319412: RISC-V: Simple fix of indent in c2_MacroAssembler_riscv.hpp
• df599db: 8295159: DSO created with -ffast-math breaks Java floating-point arithmetic
• c099cf5: 8318422: Allow poller threads be virtual threads
• 29cf2c4: 8319053: Segment dump files remain after parallel heap dump on Windows
• ea6a88a: 8318466: Improve spec of NumberFormat's methods with unsupported operations
• de6667c: 8225377: type annotations are not visible to javac plugins across compilation boundaries
• 008ca2a: 8317620: Build JDK tools with ModuleMainClass attribute
• 1a21c1a: 8318736: com/sun/jdi/JdwpOnThrowTest.java failed with "transport error 202: bind failed: Address already in use"
• 81db172: 8318955: Add ReleaseIntArrayElements in Java_sun_awt_X11_XlibWrapper_SetBitmapShape XlbWrapper.c to early return
• ... and 52 more: https://git.openjdk.org/jdk/compare/e05cafda78a37dbeb2df2edd791be19d22edaece...master

Your commit was automatically rebased without conflicts.

[openjdk]

@robcasloz Pushed as commit 377138c.

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