8263075: C2: simplify anti-dependence check in PhaseCFG::implicit_null_check()

[brianjstafford]

The reporter for this issue (https://bugs.openjdk.java.net/browse/JDK-8263075) indicated that there's an assumption that we can rely on that the while loop in question will run exactly one time. Based on this, I've done the following:

• Asserted the condition that makes sure the code runs at least once
• Asserted the condition that makes sure the code runs only once
• Removed the while loop
• Changed a couple of break statements into continue statements. They no longer need to break out of the while loop, now that it's gone. However, they were early exits from the while loop that ended up resulting in continue statements for the larger enclosing loop. Thus we can just call continue directly.
• Removed the local variable b, as we no longer need to traverse the node hierarchy. We can use mb directly.

Passes jdk, langtools, and hotspot Tier 1 tests on Linux (x64 and ARM64) and macOS (x64 and ARM64). Most Tier 1 tests pass on Windows (x64 and ARM64), but there are a handful of failures unrelated to this change.

---

Progress

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

Issue

• JDK-8263075: C2: simplify anti-dependence check in PhaseCFG::implicit_null_check()

Reviewers

• Vladimir Kozlov (@vnkozlov - Reviewer) ⚠️ Review applies to c055174a
• Tobias Hartmann (@TobiHartmann - Reviewer) ⚠️ Review applies to c055174a
• Roberto Castañeda Lozano (@robcasloz - Committer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 8684

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

Using diff file

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

[bridgekeeper]

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

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

• 04: Full - Incremental (7abf914a)
• 03: Full - Incremental (df661be7)
• 02: Full - Incremental (8129fe21)
• 01: Full - Incremental (c055174a)
• 00: Full (6de40386)

[vnkozlov]

Good.
Tobias is running testing. Please wait results.

[openjdk]

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

8263075: C2: simplify anti-dependence check in PhaseCFG::implicit_null_check()

Reviewed-by: kvn, thartmann, rcastanedalo

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

• e990fec: 8287089: G1CollectedHeap::is_in_cset() can be const methods
• 81d7eaf: 8287104: AddressChangeListener thread inherits CCL and can cause memory leak for webapp-servers
• 796494d: 8284944: assert(cnt++ < 40) failed: infinite cycle in loop optimization
• e534c13: 8286398: Address possibly lossy conversions in jdk.internal.le
• e21b527: 8228990: JFR: TestNetworkUtilizationEvent.java expects 2+ Network interfaces on Linux but finding 1
• e9bddc1: 8262889: Compiler implementation for Record Patterns
• ebfa27b: 8284690: [macos] VoiceOver : Getting java.lang.IllegalArgumentException: Invalid location on Editable JComboBox
• e32377e: 8286304: Removal of diagnostic flag GCParallelVerificationEnabled
• e1f140d: 8287206: Use WrongThreadException for confinement errors
• e17118a: 8272791: java -XX:BlockZeroingLowLimit=1 crashes after 8270947
• ... and 292 more: https://git.openjdk.java.net/jdk/compare/d8f9686b123bc9f0521da0cd286726c3b4327abd...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, @TobiHartmann, @robcasloz) 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).

[TobiHartmann]

Looks good to me and tests passed. @robcasloz should also have a look.

[robcasloz]

Looks good to me and tests passed. @robcasloz should also have a look.

Running some additional tests, will come back with the results.

[robcasloz]

Thanks for working on this cleanup, @brianjstafford! The loop removal looks good to me, and the additional tests passed, I just have a few comments about the assertions and their documentation.

[brianjstafford]

Thank you @robcasloz for the suggestions, hopefully I've incorporated them as you expected. Please let me know if I should make further changes.

[robcasloz]

Thanks for addressing all my comments! I just have a last, style comment on the updated revision.

[robcasloz]

Thanks for addressing my last comment!

[robcasloz]

@brianjstafford feel free to mark this PR as ready for integration (/integrate) when you think it is.

[brianjstafford]

The langtools/tier1 failures on Windows x64 are not related to this recent whitespace change, and I'm seeing similar failures on other outstanding PRs. Might be an infrastructure issue. However, I'm not positive if the failures will prevent integration.

[brianjstafford]

/integrate

[openjdk]

@brianjstafford
Your change (at version 7abf914) is now ready to be sponsored by a Committer.

[vnkozlov]

/sponsor

[openjdk]

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

• 0b3d409: 8261768: SelfDestructTimer should accept seconds
• bc0379e: 8275303: sun/java2d/pipe/InterpolationQualityTest.java fails with D3D basic render driver
• 0b8dd4a: 8284966: Update SourceVersion.RELEASE_19 description for language changes
• e990fec: 8287089: G1CollectedHeap::is_in_cset() can be const methods
• 81d7eaf: 8287104: AddressChangeListener thread inherits CCL and can cause memory leak for webapp-servers
• 796494d: 8284944: assert(cnt++ < 40) failed: infinite cycle in loop optimization
• e534c13: 8286398: Address possibly lossy conversions in jdk.internal.le
• e21b527: 8228990: JFR: TestNetworkUtilizationEvent.java expects 2+ Network interfaces on Linux but finding 1
• e9bddc1: 8262889: Compiler implementation for Record Patterns
• ebfa27b: 8284690: [macos] VoiceOver : Getting java.lang.IllegalArgumentException: Invalid location on Editable JComboBox
• ... and 295 more: https://git.openjdk.java.net/jdk/compare/d8f9686b123bc9f0521da0cd286726c3b4327abd...master

Your commit was automatically rebased without conflicts.

[openjdk]

@vnkozlov @brianjstafford Pushed as commit c674348.

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