-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
8253588: C1: assert(false) failed: unknown register on x86_32 only with -XX:+TraceLinearScanLevel=4 #356
Conversation
…2 only with -XX:+TraceLinearScanLevel=4
👋 Welcome back chagedorn! A progress list of the required criteria for merging this PR into |
@chhagedorn The following label will be automatically applied to this pull request: 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks reasonable to me.
@chhagedorn 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:
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 237 new commits pushed to the
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good.
Thank you Tobias and Vladimir for your reviews! |
/integrate |
@chhagedorn Since your change was applied there have been 250 commits pushed to the
Your commit was automatically rebased without conflicts. Pushed as commit 13fe054. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
JDK-8251093 introduced some additional logging of intervals and its registers in various places. On 32-bit only, we could have two registers for an interval. A hi-register is only used when the interval has
_num_phys_regs
set to 2. In one such place (L5448), we log the hi-registerhint_regHi
. On L5441, however, we can assign it an invalid register number when_num_phys_regs
is 1. That was not a problem before JDK-8251093 as we only usedhint_regHi
later after a_num_phys_regs == 2
check on L5484. But the additional logging is performed earlier resulting in this assertion failure when trying to log the invalidhint_regHi
register.Thanks,
Christian
Progress
Issue
Reviewers
Download
$ git fetch https://git.openjdk.java.net/jdk pull/356/head:pull/356
$ git checkout pull/356