8267265: Use new IR Test Framework to create tests for C2 Ideal transformations

[JohnTortugo]

Hi, can I please get some reviews for this Pull Request? Here is a summary of the changes:

• Add tests, using the new IR-based test framework, for several of the Ideal transformations on Add, Sub, Mul, Div, Loop nodes and some simple Scalar Replacement transformations.
• Add more default IR regex's to IR-based test framework.

/cc @chhagedorn - I decided to create a new PR for this work since the previous one was quite stale. I believe I've addressed all feedback that you provided so far, please let me know if there is something missing and I'll get to it asap.

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change must be properly reviewed

Issue

• JDK-8267265: Use new IR Test Framework to create tests for C2 Ideal transformations

Reviewers

• Christian Hagedorn (@chhagedorn - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/7434/head:pull/7434
$ git checkout pull/7434

Update a local copy of the PR:
$ git checkout pull/7434
$ git pull https://git.openjdk.java.net/jdk pull/7434/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 7434

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/7434.diff

[bridgekeeper]

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

@JohnTortugo Usage: /cc <add|remove> [label[, label, ...]] or /cc [<+|->label[, <+|->label, ...]] where label is an additional classification that should be applied to this PR. These labels are valid:

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

[openjdk]

@JohnTortugo 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

• 02: Full - Incremental (61d0a1a7)
• 01: Full - Incremental (8c66e0e5)
• 00: Full (88bfba36)

[vnkozlov]

Nice. I will run testing to make sure they work in all configurations (which we testing).

[vnkozlov]

I got compilation failures for new tests. For example:

test/hotspot/jtreg/compiler/c2/irTests/SubLNodeIdealizationTests.java:83: error: cannot find symbol
    @IR(failOn = {IRNode.SUB})
                        ^
  symbol:   variable SUB
  location: class IRNode

[chhagedorn]

Nice work! Thanks for coming back to this with the updates. I'll have a closer look next week.

[chhagedorn]

Apart from some minor things and some possible missing cases when comparing the int vs long tests, it looks good to me!

[chhagedorn]

Looks good, thanks for doing the updates!

I'll submit another round of testing over the weekend.

[openjdk]

⚠️ @JohnTortugo the full name on your profile does not match the author name in this pull requests' HEAD commit. If this pull request gets integrated then the author name from this pull requests' HEAD commit will be used for the resulting commit. If you wish to push a new commit with a different author name, then please run the following commands in a local repository of your personal fork:

$ git checkout ir-tests
$ git commit -c user.name='Preferred Full Name' --allow-empty -m 'Update full name'
$ git push

[openjdk]

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

8267265: Use new IR Test Framework to create tests for C2 Ideal transformations

Reviewed-by: chagedorn

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

• 2557ef8: 8282276: Problem list failing two Robot Screen Capture tests
• 6445ee4: 5041655: (ch) FileLock: negative param and overflow issues
• 7feabee: 8261407: ReflectionFactory.checkInitted() is not thread-safe
• 58e1882: 8282042: [testbug] FileEncodingTest.java depends on default encoding
• 3cb3867: 8281315: Unicode, (?i) flag and backreference throwing IndexOutOfBounds Exception
• 957dae0: 8280958: G1/Parallel: Unify marking code structure
• e44d067: 8244593: Clean up GNM/NM after JEP 381
• 41355e2: 8276686: Malformed Javadoc inline tags in JDK source in /java/util/regex/Pattern.java
• 022d807: 8271008: appcds/*/MethodHandlesAsCollectorTest.java tests time out because of excessive GC (CodeCache GC Threshold) in loom
• ab6d8e6: 8260328: Drop redundant CSS properties from java.desktop HTML files
• ... and 123 more: https://git.openjdk.java.net/jdk/compare/83b6e4bc04db89a846a1b6c2d0666efe139f8f61...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.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@vnkozlov, @chhagedorn) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

[chhagedorn]

I'll submit another round of testing over the weekend.

hs-tier1-3 looked good!

[JohnTortugo]

/integrate

[openjdk]

@JohnTortugo
Your change (at version 61d0a1a) is now ready to be sponsored by a Committer.

[chhagedorn]

/sponsor

[openjdk]

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

• 86723d4: 8281507: Two javac tests have bad jtreg @clean tags
• 630ad1a: 8282428: ProblemList jdk/jfr/jvm/TestWaste.java
• afd4bcb: 8282398: EndingDotHostname.java test fails because SSL cert expired
• cf6d256: 8282153: JFR: Check for recording waste
• c5c6058: 8282219: jdk/java/lang/ProcessBuilder/Basic.java fails on AIX
• fb8bf81: 8279995: jpackage --add-launcher option should allow overriding description
• 441e485: 8281739: JFR: Use message with Objects.requireNonNull
• e96c599: 8271232: JFR: Scrub recording data
• 735e86b: 8282345: handle latest VS2022 in abstract_vm_version
• b96b743: 8281015: Further simplify NMT backend
• ... and 167 more: https://git.openjdk.java.net/jdk/compare/83b6e4bc04db89a846a1b6c2d0666efe139f8f61...master

Your commit was automatically rebased without conflicts.

[openjdk]

@chhagedorn @JohnTortugo Pushed as commit efd3967.

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

[JohnTortugo]

Thank you @chhagedorn !