8269752: C2: assert(false) failed: Bad graph detected in build_loop_late #247
Conversation
|
|
Webrevs
|
| @@ -2933,7 +2933,7 @@ bool PhaseIdealLoop::multi_version_post_loops(IdealLoopTree *rce_loop, IdealLoop | |||
| } | |||
|
|
|||
| // Find RCE'd post loop so that we can stage its guard. | |||
| if (!is_canonical_loop_entry(legacy_cl)) return multi_version_succeeded; | |||
| if (legacy_cl->is_canonical_loop_entry() == NULL) return multi_version_succeeded; | |||
There was a problem hiding this comment.
Can you also add braces here?
|
@rwestrel This change now passes all automated pre-integration checks. 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 19 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.
|
|
@chhagedorn thanks for the review |
| } | ||
| // compares can get conditionally flipped | ||
|
|
||
| int input = is_main_loop() ? 2 : 1; | ||
| bool res = cmpzm->in(input)->Opcode() == Op_Opaque1; |
There was a problem hiding this comment.
You need to check input < cmpzm->in(input) and cmpzm->in(input) for NULL. There could be a "dead" path.
There was a problem hiding this comment.
Thanks for reviewing this. I updated the patch.
|
/integrate |
|
Going to push as commit c130451.
Your commit was automatically rebased without conflicts. |
Some nodes are sunk out of the outer loop of vMeth(). A CastII node is
added on the exit path of the loop to pin the nodes out of the
loop. The dominating test of that CastII is:
if (i >= 132) {
so the CastII is updated with type int [132..]
The inner loop is eliminated, the outer loop is turned into a counted
loop, pre/main/post loops are created. The CastII control is now a
Region that merges paths from the pre/main/post loops. Its data input
is a Phi that merges the iv Phis of the 3 loops.
The loop body includes an early exit:
if (i2 != 0) {
vMeth_check_sum += i3;
return;
}
That test is guaranteed to be taken at the second iteration. As a
consequence, the backedge of the main loop is removed. What's left of
the main loop is still guarded by a an entry test that compares the iv
value out of the pre loop with an Opaque1 node. The CastII is now only
reachable through that test. The iv Phi of the pre loop constant folds
to a value that's not >= 132. As a consequence the CastII becomes
top. top propagates to other data node and as a consequence a Phi is
replaced by its only non top input. That causes the assert failure.
The CastII should not be reachable anymore once the iv Phi of the pre
loop has constant folded and the early exit in the main loop is always
taken. But because the test that guards the main loop uses an Opaque1,
it cannot constant fold. As a fix, I propose removing the Opaque1 once
the backedge of the main loop is removed. This allows the entry test
to constant fold. The Opaque1 has no purpose anymore given the loop
doesn't exist at this point.
Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk17 pull/247/head:pull/247$ git checkout pull/247Update a local copy of the PR:
$ git checkout pull/247$ git pull https://git.openjdk.java.net/jdk17 pull/247/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 247View PR using the GUI difftool:
$ git pr show -t 247Using diff file
Download this PR as a diff file:
https://git.openjdk.java.net/jdk17/pull/247.diff