valhalla
valhalla copied to clipboard
8239003: [lworld] C2 should respect larval state when scalarizing
Hi All,
Patch addresses issues around unsafe updates to value objects within a loop and larval state preservation by suppressing scalarization of value objects in larval state.
Since Unsafe.put* APIs returns a void, hence it does not alter JVM state. Due to this ciTypeFlow dataflow analysis does not encounter an inductive definition corresponding to updated value object within the loop, due to this C2 parser misses creating an inductive phi node on encountering loop header block.
In order to maintain IR compliance with ciTypeFlow analysis C2 should prevent scalarizing value object b/w make and finish private buffer calls.
New behavior of unsafe inline expanders
- makePrivate: Receive InlineTypeNode input and return initialized buffer in larval state.
- finishPrivateBuffer: Receive value object buffer input and return rematerialize InlineTypeNode in non-larval state. This may result into creation of unbalanced phi node at control flow merges where one of the phi input may InlineTypeNode and other is a buffer of compatible value type, but the IR still remains valid.
In addition compiler is now preventing elimination of allocation in larval state.
Validation Status:-
All the Valhalla JTREG tests are passing at various AVX level with / wo -XX:+DoptimizeALot.
Kindly review and share your feedback.
Best Regards, Jatin
Progress
- [ ] Change must not contain extraneous whitespace
Issue
- JDK-8239003: [lworld] C2 should respect larval state when scalarizing (Bug - P2)
Reviewing
Using git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/valhalla.git pull/964/head:pull/964
$ git checkout pull/964
Update a local copy of the PR:
$ git checkout pull/964
$ git pull https://git.openjdk.org/valhalla.git pull/964/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 964
View PR using the GUI difftool:
$ git pr show -t 964
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/valhalla/pull/964.diff
Webrev
:wave: Welcome back jbhateja! 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.
@jatin-bhateja This change is no longer ready for integration - check the PR body for details.
Thanks for working on this, Jatin. As you stated in the graph in your latest comment, handling both the larvae and the non-larvae cases in C2 compiled code would require method multiversioning, so it's not an option for now. I followed up in the email thread that you started.
@jatin-bhateja this pull request can not be integrated into lworld due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:
git checkout JDK-8239003
git fetch https://git.openjdk.org/valhalla.git lworld
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge lworld"
git push
@jatin-bhateja This pull request has been inactive for more than 8 weeks and will be automatically closed if another 8 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!
@jatin-bhateja This pull request has been inactive for more than 16 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open pull request command.