8314319: LogCompilation doesn't reset lateInlining when it encounters a failure.#15375
8314319: LogCompilation doesn't reset lateInlining when it encounters a failure.#15375navyxliu wants to merge 2 commits intoopenjdk:masterfrom
Conversation
|
👋 Welcome back xliu! A progress list of the required criteria for merging this PR into |
|
Hi, |
ericcaspole
left a comment
There was a problem hiding this comment.
Looks good, thanks for figuring this out.
vnkozlov
left a comment
There was a problem hiding this comment.
Looks good.
I ran tier1 testing to make sure it passed our source code validation (for your new test). It passed.
|
@navyxliu 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 130 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 |
|
thank you for reviewing this! |
|
/integrate |
|
Going to push as commit e9e0c56.
Your commit was automatically rebased without conflicts. |
This patch fixed a bug in LogCompilation. A compilation may encounter a failure after it processes
'<late_inline>' tag. Sometimes, C2 compiler would retry after tweaking options. In this case, it would retry it
without subsume_load. If we don't reset lateInlining, we may have trouble in the retry run.
We also develop a unittest to verify that. A strip jit.xml is placed in test/resources/ directory.
It's worth noting that 'mvn test' reports the 2 tests passed even without this patch. We can see the stacktrace
of exceptions. This isn't an accident. There are 2 reasons:
I am not sure they are by design. I manage to fix those 2 problems, but fixing them is beyond the scope of this
patch. I would like to hear reviewer's feedbacks first.
Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/15375/head:pull/15375$ git checkout pull/15375Update a local copy of the PR:
$ git checkout pull/15375$ git pull https://git.openjdk.org/jdk.git pull/15375/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 15375View PR using the GUI difftool:
$ git pr show -t 15375Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/15375.diff
Webrev
Link to Webrev Comment