8288421: IGV: warn user about all unreachable nodes

[robcasloz]

This changeset ensures that, when approximating C2's schedule, IGV does not schedule unreachable nodes. Instead, a node warning is emitted, informing the user that the corresponding node is unreachable. This information can be useful when debugging ill-formed graphs.

The following clustered subgraph illustrates the proposed change:

Currently (before), 522 IfFalse gets assigned the same block as 256 Region (B11) in an effort to schedule as many nodes as possible, and hence no warning is emitted for 522 IfFalse, even though it is clearly control-unreachable (since it is a child of 520 If which is control-unreachable). This changeset (after) leaves instead 522 IfFalse unscheduled and emits a "Control-unreachable CFG node" warning for it (visible as a tooltip of the node warning sign).

As a side-benefit, the changeset simplifies the IGV scheduling algorithm by removing the code that tries to schedule unrechable nodes code on a best-effort basis, and adds two additional node warnings ("Region with multiple successors" and "CFG node without control successors") to highlight the new nodes that might remain unscheduled as a consequence.

Testing

• Tested manually on the graph attached to the JBS issue.

• Tested automatically that scheduling tens of thousands of graphs (by instrumenting IGV to schedule parsed graphs eagerly and running java -Xcomp -XX:-TieredCompilation -XX:PrintIdealGraphLevel=4) does trigger any exception or assertion failure.

---

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-8288421: IGV: warn user about all unreachable nodes

Reviewers

• Christian Hagedorn (@chhagedorn - Reviewer)
• Tobias Hartmann (@TobiHartmann - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 9164

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

Using diff file

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

[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

• 00: Full (a7a682d9)

[chhagedorn]

Looks good, thanks for fixing this! And nice new warnings.

[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:

8288421: IGV: warn user about all unreachable nodes

Reviewed-by: chagedorn, 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 12 new commits pushed to the master branch:

• 6633855: 8288399: MacOS debug symbol files not always deterministic in reproducible builds
• d5cd2f2: 8284849: Add deoptimization to unified logging
• dfeeb6f: 8288140: Avoid redundant Hashtable.get call in Signal.handle
• 68b2057: 8287373: remove unnecessary paddings in generated code
• 2471f8f: 8287647: VM debug support: find node by pattern in name or dump
• 33f34d5: 8288207: Enhance MalformedURLException in Uri.parseCompat
• 444a0d9: 8284977: MetricsTesterCgroupV2.getLongValueEntryFromFile fails when named value doesn't exist
• 08400f1: 8287349: AArch64: Merge LDR instructions to improve C1 OSR performance
• fe80721: 8287917: System.loadLibrary does not work on Big Sur if JDK is built with macOS SDK 10.15 and earlier
• bbaeacb: 8265586: [windows] last button is not shown in AWT Frame with BorderLayout and MenuBar set.
• ... and 2 more: https://git.openjdk.org/jdk/compare/0530f4e517be5d5b3ff10be8a0764e564f068c06...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]

Looks good, thanks for fixing this! And nice new warnings.

Thanks for reviewing, Christian.

[TobiHartmann]

Looks good.

[robcasloz]

Looks good.

Thanks for reviewing, Tobias!

[robcasloz]

/integrate

[openjdk]

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

• 9d4b25e: 8288281: compiler/vectorapi/VectorFPtoIntCastTest.java failed with "IRViolationException: There were one or multiple IR rule failures."
• e833c1d: 8285519: Change usages of TimeUnit.convert to TimeUnit.toXXX
• e7d52e2: 8288594: Address possibly lossy conversions in java.base FloatToDecimal
• 2cf7c01: 8288478: AArch64: Clean up whitespace in assembler_aarch64.hpp
• ce5024f: 8288468: Avoid redundant HashMap.get call in NegotiateAuthentication.firstToken
• 8fc8fdc: 8287905: Reduce runtime of java.nio microbenchmarks
• cf4a496: 8288064: Class initialization locking
• 3d12c02: Merge
• 729164f: 8288533: Missing @param tags in com.sun.source classes
• 9254e12: 8288526: ProblemList gc/stress/TestStressG1Humongous.java on windows-x64
• ... and 31 more: https://git.openjdk.org/jdk/compare/0530f4e517be5d5b3ff10be8a0764e564f068c06...master

Your commit was automatically rebased without conflicts.

[openjdk]

@robcasloz Pushed as commit f3da7ff.

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