RFR: JDK-8272574: Crashes in PhaseIdealLoop::build_loop_late_post_work [v2]

[Tobias Hartmann]

On Tue, 17 Aug 2021 12:42:49 GMT, 王超 <github.com+25214855+casparcwang at openjdk.org> wrote:

>> Current loop predication will promote nodes(with a dependency on a later control node) to the insertion point which dominates the later control node.
>> 
>> In the following example, loopPrediction will promote node 434 to the outer loop(predicted insert point is right after node 424), and it depends on control node 207.  But node 424 dominates node 207, which means after the promotion, the cloned nodes have a control dependency on a later control node, which leads to a bad graph.
>> 
>> ![image](https://user-images.githubusercontent.com/25214855/129720970-ff65b8f4-8bef-401d-8590-54aca6de470e.png)
>> 
>> ![image](https://user-images.githubusercontent.com/25214855/129721369-4c61222b-7305-4522-9a37-e3e6e2138aa9.png)
>
> 王超 has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix version

Could you please explain in more detail how we ended up with the `400 LoadI` having a dependency on `207 IfTrue` which is below the predicate insertion point? Is it part of another predicate? I think it would help if you could add comments to the test, explaining what is going on (i.e. where we insert predicates for which range checks).

Why did you add the test to `loopstripmining/LoadSplitThruPhi.java`? It's unrelated to loop strip mining, right?

src/hotspot/share/opto/loopPredicate.cpp line 666:

> 664:   }
> 665:   if (offset && phase->is_dominator(predicate_proj, phase->get_ctrl(offset))) {
> 666:     // offset must be able to float before predicte projection node

`predicte` -> `predicate`

test/hotspot/jtreg/compiler/loopstripmining/LoadSplitThruPhi.java line 52:

> 50:         int[] indexes = new int[]{0, 2};
> 51: 
> 52:         for (int a = 0; a < (int)(a + 16); a++) {

The int cast seems useless.

test/hotspot/jtreg/compiler/loopstripmining/LoadSplitThruPhi.java line 72:

> 70: 
> 71:         for (int i = 0; i < 100; ++i) {
> 72:             Thread t = new Thread(new Runnable() {

Why do you need a new thread?

-------------

Changes requested by thartmann (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/5142