Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8261037: [lworld] Assert during C2 compilation due to inconsistent JVMState at safepoint #322

Closed
wants to merge 1 commit into from

Conversation

TobiHartmann
Copy link
Member

@TobiHartmann TobiHartmann commented Feb 5, 2021

We assert when scalarizing an inline type in a safepoint because debug_end() (-> _endoff) of the attached JVMState is larger than SafePointNode::_max. I don't think JVMStates should be shared between safepoint nodes and I intend to fix this in mainline with JDK-8261158.

Gory details:
In PhaseIdealLoop::is_counted_loop, we // Check for immediately preceding SafePoint and remove by cloning SafePointNode1 to SafePointNode2 without cloning the attached JVMState. Loop unswitching then clones the loop and therefore also clones SafePointNode2 to SafePointNode3, again without cloning the JVMState. All three safepoint nodes now share the same JVMState. An inline type is scalarized in SafePointNode2, increasing JVMState::_endoff to account for the fields. Once another inline type is scalarized in SafePointNode3, JVMState::_endoff is out of bounds of SafePointNode3::_max.

This fix refactors the code (which also serves as workaround because we now access debug_end() only after we updated it to the correct value for the current safepoint) and adds a regression test.

Best regards,
Tobias


Progress

  • Change must not contain extraneous whitespace

Issue

  • JDK-8261037: [lworld] Assert during C2 compilation due to inconsistent JVMState at safepoint

Download

$ git fetch https://git.openjdk.java.net/valhalla pull/322/head:pull/322
$ git checkout pull/322

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented Feb 5, 2021

👋 Welcome back thartmann! A progress list of the required criteria for merging this PR into lworld will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@openjdk
Copy link

@openjdk openjdk bot commented Feb 5, 2021

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

8261037: [lworld] Assert during C2 compilation due to inconsistent JVMState at safepoint

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 231 new commits pushed to the lworld branch:

  • 33dc4fb: Merge jdk
  • f025bc1: 8260301: misc gc/g1/unloading tests fails with "RuntimeException: Method could not be enqueued for compilation at level N"
  • 4a8b5c1: 8257858: [macOS]: Remove JNF dependency from libosxsecurity/KeystoreImpl.m
  • 472bf62: 8258799: [Testbug] RandomCommandsTest must check if tested directive is added via jcmd
  • b0ee7a8: 8241995: Clarify InetSocketAddress::toString specification
  • 0ef93fe: 8259265: Refactor UncaughtExceptions shell test as java test.
  • 5324b5c: 8260998: Shenandoah: Restore reference processing statistics reporting
  • c8de943: 8260617: Merge ZipFile encoding check with the initial hash calculation
  • ae2c5f0: 8260581: IGV: enhance node search
  • 9037615: 8222850: jshell tool: Misleading cascade compiler error in switch expression with undefined vars
  • ... and 221 more: https://git.openjdk.java.net/valhalla/compare/fbb8a6bdf878d2c370a0e78b04a5a5736f2eb765...lworld

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 lworld branch, type /integrate in a new comment.

@mlbridge
Copy link

@mlbridge mlbridge bot commented Feb 5, 2021

Webrevs

@TobiHartmann
Copy link
Member Author

@TobiHartmann TobiHartmann commented Feb 5, 2021

/integrate

@openjdk openjdk bot closed this Feb 5, 2021
@openjdk openjdk bot added integrated and removed ready rfr labels Feb 5, 2021
@openjdk
Copy link

@openjdk openjdk bot commented Feb 5, 2021

@TobiHartmann Since your change was applied there have been 231 commits pushed to the lworld branch:

  • 33dc4fb: Merge jdk
  • f025bc1: 8260301: misc gc/g1/unloading tests fails with "RuntimeException: Method could not be enqueued for compilation at level N"
  • 4a8b5c1: 8257858: [macOS]: Remove JNF dependency from libosxsecurity/KeystoreImpl.m
  • 472bf62: 8258799: [Testbug] RandomCommandsTest must check if tested directive is added via jcmd
  • b0ee7a8: 8241995: Clarify InetSocketAddress::toString specification
  • 0ef93fe: 8259265: Refactor UncaughtExceptions shell test as java test.
  • 5324b5c: 8260998: Shenandoah: Restore reference processing statistics reporting
  • c8de943: 8260617: Merge ZipFile encoding check with the initial hash calculation
  • ae2c5f0: 8260581: IGV: enhance node search
  • 9037615: 8222850: jshell tool: Misleading cascade compiler error in switch expression with undefined vars
  • ... and 221 more: https://git.openjdk.java.net/valhalla/compare/fbb8a6bdf878d2c370a0e78b04a5a5736f2eb765...lworld

Your commit was automatically rebased without conflicts.

Pushed as commit ce486c3.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
1 participant