RFR: JDK-8275854: C2: assert(stride_con != 0) failed: missed some peephole opt [v3]

[Tobias Hartmann]

On Tue, 26 Oct 2021 01:42:41 GMT, 王超 <duke at openjdk.java.net> wrote:

>> `If subsume` optimization will eliminate `LongCountedLoopEndNode` node by mistake, which will lead to `PhaseIdealLoop` optimization crash.
>> 
>> For example, the test of node 538 and node 553 will become the same after the first `PhaseIdealLoop` optimization. Node 555 is the back edge to the loop, and node 553 will be replaced by a `LongCountedLoopEndNode` node.
>> <img width="592" alt="image" src="https://user-images.githubusercontent.com/25214855/138656330-9f825c85-0468-40a5-9eb9-b576130648db.png">
>> 
>> 
>>  In the next `PhaseIdealLoop` optimization, node 538 find node 553 is redundant, and will subsume node 553. Then the `PhaseIdealLoop` optimization will crash, because there is no loop end node.
>> <img width="603" alt="image" src="https://user-images.githubusercontent.com/25214855/138656951-53685723-92c2-42d6-91ae-445107dcb01d.png">
>> 
>> There are two way to fix the crash, the first is like the way in this pr, just exit `IFNode subsume` optimization when it's a `LongCountedLoopEndNode` node. The second possible fix is that exchange the dominating `IF` node with the `LongCountedLoopEndNode` node:
>> 
>> diff --git a/src/hotspot/share/opto/ifnode.cpp b/src/hotspot/share/opto/ifnode.cpp
>> index 38b40a6..31ff172 100644
>> --- a/src/hotspot/share/opto/ifnode.cpp
>> +++ b/src/hotspot/share/opto/ifnode.cpp
>> @@ -1674,6 +1674,21 @@ Node* IfNode::simple_subsuming(PhaseIterGVN* igvn) {
>>      }
>>    }
>>  
>> +  if (is_LongCountedLoopEnd()) {
>> +    set_req(0, dom->in(0));
>> +    set_req(1, dom->in(1));
>> +    dom->set_req(0, pre);
>> +    dom->set_req(1, igvn->intcon(is_always_true ? 1 : 0));
>> +    Node* proj0 = raw_out(0);
>> +    Node* proj1 = raw_out(1);
>> +    Node* dom_proj0 = dom->raw_out(0);
>> +    Node* dom_proj1 = dom->raw_out(1);
>> +    dom_proj0->set_req(0, this);
>> +    dom_proj1->set_req(0, this);
>> +    proj0->set_req(0, dom);
>> +    proj1->set_req(0, dom);
>> +  }
>> +
>>    if (bol->outcnt() == 0) {
>>      igvn->remove_dead_node(bol);    // Kill the BoolNode.
>>    }
>> diff --git a/src/hotspot/share/opto/loopnode.cpp b/src/hotspot/share/opto/loopnode.cpp
>> index 6f7e34d..7955722 100644
>> --- a/src/hotspot/share/opto/loopnode.cpp
>> +++ b/src/hotspot/share/opto/loopnode.cpp
>> @@ -802,7 +802,7 @@ bool PhaseIdealLoop::transform_long_counted_loop(IdealLoopTree* loop, Node_List
>>    Node* back_control = head->in(LoopNode::LoopBackControl);
>>  
>>    // data nodes on back branch not supported
>> -  if (back_control->outcnt() > 1) {
>> +  if (back_control->outcnt() > 1 || back_control->Opcode() != Op_IfTrue) {
>>      return false;
>>    }
>
> 王超 has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Adjust the code style

The problem with such large and non-targeted regression tests is that they won't work for long. Other changes to C2 and/or HotSpot will change timing, profile information, IR shape, optimization sequence or other factors such that the issue will not reproduce anymore with that test. Often, the test also does not reproduce the issue in older JDK versions that are affected as well.

We therefore usually run `creduce --not-c` on our generated tests to simplify them (see [creduce](https://embed.cs.utah.edu/creduce/)). You might want to increase the number of loop iterations in the main method first and also add `-Xbatch`.

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

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