8360175: C2 crash: assert(edge_from_to(prior_use,n)) failed: before block local scheduling

[mhaessig]

The triggered assert is part of the schedule verification code that runs just before machine code is emitted. The debug output showed that a leaPCompressedOopOffset node was causing the assert, which suggested the peephole optimization introduced in #25471 as the cause. The failure proved quite difficult to reproduce. It failed more often on Windows and required -XX:+UseKNLSetting (forces code generation for Intel's Knights Landing platform), which forces -XX:+OptoScheduling.

The root-cause is a subtle bug in the rewiring of the base edge of leaP* nodes in the remove_redundant_lea peephole. When the peephole removed a decodeHeapOop_not_null including a spill, it did not set the base edge of the leaP* node to the same node as the address edge, which is the intent of the peephole, but to the parent node of the spill. That is not catastrophic in most cases, but might reference another register slot, which causes this assert. Concretely, we see the following graph

    MemToRegSpillCopy
     |             |
     |    MemToRegSpillCopy
     |             |    
DefiniinoSpillCopy |
     |             |
     |  decodeHeapOop_not_null
     |             |
   leaPCompressedHeapOop

gets rewired to

     MemToRegSpillCopy
       |            |    
DefinitionSpillCopy |
       |            |
   leaPCompressedHeapOop

instead of

  MemToRegSpillCopy
         |
 DefinitionSpillCopy
        / \     
leaPCompressedHeapOop

This PR fixes this by always setting the base edge of the leaP* node to the same node as the address edge. Unfortunately, I was not able to construct a regression test because of the difficulty of reproducing the bug.

Testing

• Github Actions
• tier1,tier2 plus internal testing on all Oracle supported platforms
• tier3,tier4,tier5 plus internal testing on Linux and Windows x64
• Runthese8H on windows-x64-debug (test that reliably produced the failure addressed in this PR)

---

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-8360175: C2 crash: assert(edge_from_to(prior_use,n)) failed: before block local scheduling (Bug - P3)

Reviewers

• Vladimir Kozlov (@vnkozlov - Reviewer)
• Christian Hagedorn (@chhagedorn - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 26157

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

Using diff file

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

Using Webrev

Link to Webrev Comment

[bridgekeeper]

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

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

8360175: C2 crash:  assert(edge_from_to(prior_use,n)) failed: before block local scheduling

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

• 310ef85: 8305567: serviceability/tmtools/jstat/GcTest01.java failed utils.JstatGcResults.assertConsistency
• 5205eae: 8346753: Test javax/swing/JMenuItem/RightLeftOrientation/RightLeftOrientation.java fails on Windows Server 2025 x64 because the icons of RBMenuItem and CBMenuItem are not visible in Nimbus LookAndFeel
• bbc5c98: 8342868: Errors related to unused code on Windows after 8339120 in core libs
• ... and 13 more: https://git.openjdk.org/jdk/compare/d75ea7e67951275fe27f1e137c961f39d779a046...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.

[openjdk]

@mhaessig 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 (b6fa1f6c)

[vnkozlov]

Seems fine.

[chhagedorn]

The fix looks reasonable to me, too. No worries about the regression test, thanks for trying! A small question: Why don't we use lea_address?

Another thing I've noticed while browsing the code: ra_ and new_root seem to be unused and could be removed (could probably also be squeezed into this PR here instead of creating a new issue just for that).

[mhaessig]

We cannot use lea_address because in case of a spill that also gets moved up one node to check if the lea and the decode point to the same grandparent.

Another thing I've noticed while browsing the code: ra_ and new_root seem to be unused and could be removed.

These arguments come from the machinery that calls this out of the matcher. I am not too familiar with it, so my working assumption so far has been to keep the signature the same as the other peepholes, which seems logical since it is called by generated code.