8263189: C2: assert(!had_error) failed: bad dominance #3005
Conversation
|
|
|
Changes seems fine. |
Thanks for the review. for both int and long counted loops. I noticed (when I added this) that after the CountedLoop is built, we can narrow the iv Phi type but often, the loop is transformed in the same loop opts round (pre/main/post loops added) before igvn has a chance to run. When igvn finally runs, it can't narrow the iv Phi anymore. Running igvn there for both int and long counted loops would seem quite disruptive. |
|
@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 164 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.
|
|
@TobiHartmann thanks for the review |
|
/integrate |
openjdk
bot
added
integrated
|
@rwestrel Since your change was applied there have been 166 commits pushed to the
Your commit was automatically rebased without conflicts. Pushed as commit fd3a33a. |
A long counted loop has a Phi that depends on the iv
Phi. PhaseIdealLoop::is_counted_loop() is able to compute a narrow
type for the iv Phi (the loop only runs for a few iterations). As a
consequence the Phi that depends on the iv Phi also gets a narrow
type. When the long counted loop is transformed into a loop nest, even
though the long loop is known to run for only a few iterations, the
inner int counted loop that's created gets a wide type: the narrow
type of the long counted loop iv Phi should result in a narrow type
for the int counted loop iv Phi but doesn't. C2 then creates
pre/main/post loops for the inner int counted loop. It wouldn't have
proceeded with this step, had the number of iterations of the loop not
been lost. In the process, it creates Phi to merge the values from the
pre/main/post loop. Two of those Phis are for the dependent Phi, the
one that captured the narrow type. The type of one of the 2 Phis
becomes top because we get in an impossible situation where the loop
that's known to have very few iterations was cloned twice. One of the
just created Phi becomes top and dominance is broken.
The reason the iv Phi narrow type is lost at long loop transformation
time is because it's queried from the igvn but it was not yet recorded
with the igvn and instead set when the long counted loop was created
in the same loop pass. The fix for that is to query the type stored in
the Phi rather than from the igvn.
Now it's not right that we end up with a dependent Phi that captures
the narrow type (which happens at igvn time) before the loop is even
created (I said that happens in the same loop opts pass so igvn has
not had a chance to run). The reason for that is that the same loop is
transformed multiple times:
1- the long counted loop is created
2- the long counted loop is transformed to a loop nest with an inner
int counted loop. The outer loop is transformed back to a regular
loop.
3- the inner int counted loop is found to be empty and so optimized out
4- the long counted loop for the same loop is created again
5- a loop nest is created again
This looks wrong and I added code to prevent retransforming a long
counted loop.
Progress
Issue
Reviewers
Download
To checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/3005/head:pull/3005$ git checkout pull/3005To update a local copy of the PR:
$ git checkout pull/3005$ git pull https://git.openjdk.java.net/jdk pull/3005/head