8367627: C2: Missed Ideal() optimization opportunity with MemBar #28448
+80
−0
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR addresses a missed optimization in
PhaseIterGVNforMemBarAcquirenodes caused by a missing notification during parsing.The missed optimization in question is the removal of the the
in(MemBarNode::Precedent)edge forMemBarAcquirenodes when the theMemBaris the only user of its input. This was intiallyintroduced to get rid of unused
Loadnodes that were only kept alive by such an edge.jdk/src/hotspot/share/opto/memnode.cpp
Lines 4254 to 4259 in eeb7c3f
In our case, it happens that the
Loadnode gets folded to a constant during the initial_gvn.transformcall inGraphKit::make_load. Because the value is converted before beingreturned, we end up with two constant nodes: one
ConLand oneConI. TheConLonlyhas one usage, and this triggers the optimization during verification.
The optimization is not triggered earlier during when we apply
_gvn.transformon the membar,because it requires
can_reshape, which is set tofalsein when we callapply_idealinPhaseGVN::transform.For this reason, we should call
record_for_igvn(membar)after theMemBaris createdand transformed in
GraphKit::insert_mem_barto make sure it gets anIdealpass withcan_reshapelater.This issue was initially filed for Valhalla, because a condition in
GraphKit::make_loadprevents its from occurring when boxing elimination is enabled. Boxing elimination is
disabled temporarily in Valhalla (see JDK-8328675),
which caused the issue to appear, but by using
-XX:-EliminateAutoBoxit became clearthat the issue was on mainline.
Testing
Thank you for reviewing!
Progress
Issue
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/28448/head:pull/28448$ git checkout pull/28448Update a local copy of the PR:
$ git checkout pull/28448$ git pull https://git.openjdk.org/jdk.git pull/28448/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 28448View PR using the GUI difftool:
$ git pr show -t 28448Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/28448.diff
Using Webrev
Link to Webrev Comment