-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
8332369: C2: assert(false) failed: graph should be schedulable after JDK-8324517 #19277
base: master
Are you sure you want to change the base?
Conversation
👋 Welcome back roland! A progress list of the required criteria for merging this PR into |
@rwestrel This change is no longer ready for integration - check the PR body for details. |
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.
That looks reasonable. I've submitted some testing.
// Check if a%b and a/b both exist | ||
Node* d = n->find_similar(Op_DivI); | ||
if (d) { | ||
if (d && !d->has_prec_edges()) { |
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.
Could be replaced with
if (d && !d->has_prec_edges()) { | |
if (d != nullptr && !d->has_prec_edges()) { |
Same at other places below.
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 good to me too. Christian's testing passed.
@rwestrel this pull request can not be integrated into git checkout JDK-8332369
git fetch https://git.openjdk.org/jdk.git master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push |
The issue occurs when a
Mod
node is processed duringfinal_graph_reshaping: if a
Div
node is found with the same inputs,the
Mod
is replaced either by aDivMod
node or a subgraph that hasthe
Div
node as input. Finding theDiv
node is donefind_similar()
which ignores the precedence edges. What happens isthat the
Div
node returned byfind_similar()
could have aprecedence edge that pins it at a control that doesn't dominate the
control of some of the uses of the
Mod
node.The fix I propose is to simply not perfom the transformation if one of
the nodes has precedence edges (which should be a rare corner case).
Progress
Issue
Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/19277/head:pull/19277
$ git checkout pull/19277
Update a local copy of the PR:
$ git checkout pull/19277
$ git pull https://git.openjdk.org/jdk.git pull/19277/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 19277
View PR using the GUI difftool:
$ git pr show -t 19277
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/19277.diff
Webrev
Link to Webrev Comment