[jdk19] RFR: 8289954: C2: Assert failed in PhaseCFG::verify() after JDK-8183390

[Pengfei Li]

On Tue, 26 Jul 2022 08:34:42 GMT, Fei Yang <fyang at openjdk.org> wrote:

>> Fuzzer tests report an assertion failure issue in C2 global code motion
>> phase. Git bisection shows the problem starts after our fix of post loop
>> vectorization (JDK-8183390). After some narrowing down work, we find it
>> is caused by below change in that patch.
>> 
>> 
>> @@ -422,14 +404,7 @@
>>        cl->mark_passed_slp();
>>      }
>>      cl->mark_was_slp();
>> -    if (cl->is_main_loop()) {
>> -      cl->set_slp_max_unroll(local_loop_unroll_factor);
>> -    } else if (post_loop_allowed) {
>> -      if (!small_basic_type) {
>> -        // avoid replication context for small basic types in programmable masked loops
>> -        cl->set_slp_max_unroll(local_loop_unroll_factor);
>> -      }
>> -    }
>> +    cl->set_slp_max_unroll(local_loop_unroll_factor);
>>    }
>>  }
>> 
>> 
>> This change is in function `SuperWord::unrolling_analysis()`. AFAIK, it
>> helps find a loop's max unroll count via some analysis. In the original
>> code, we have loop type checks and the slp max unroll value is set for
>> only some types of loops. But in JDK-8183390, the check was removed by
>> mistake. In my current understanding, the slp max unroll value applies
>> to slp candidate loops only - either main loops or RCE'd post loops -
>> so that check shouldn't be removed. After restoring it we don't see the
>> assertion failure any more.
>> 
>> The new jtreg created in this patch can reproduce the failed assertion,
>> which checks `def_block->dominates(block)` - the domination relationship
>> of two blocks. But in the case, I found the blocks are in an unreachable
>> inner loop, which I think ought to be optimized away in some previous C2
>> phases. As I'm not quite familiar with the C2's global code motion, so
>> far I still don't understand how slp max unroll count eventually causes
>> that problem. This patch just restores the if condition which I removed
>> incorrectly in JDK-8183390. But I still suspect that there is another
>> hidden bug exists in C2. I would be glad if any reviewers can give me
>> some guidance or suggestions.
>> 
>> Tested hotspot::hotspot_all_no_apps, jdk::tier1~3 and langtools::tier1.
>
> Sorry, but seems that the same assertion failure is still happening when running the newly added test case with fastdebug build on linux-riscv64 platform. And I have attached the hs_err and reply files on the JBS issue. Please take another look.

Thanks @RealFYang for the information. I'm still investigating this in jdk 20 but so far I haven't got a clear clue. Just find if I ban `SuperWord::unrolling_analysis()` for normal loops, specifically with below change, that assertion fails again.

diff --git a/src/hotspot/share/opto/superword.cpp b/src/hotspot/share/opto/superword.cpp
index ef66840628f..997fae367b1 100644
--- a/src/hotspot/share/opto/superword.cpp
+++ b/src/hotspot/share/opto/superword.cpp
@@ -217,6 +217,7 @@ void SuperWord::unrolling_analysis(int &local_loop_unroll_factor) {
   int *ignored_loop_nodes = NEW_RESOURCE_ARRAY(int, ignored_size);
   Node_Stack nstack((int)ignored_size);
   CountedLoopNode *cl = lpt()->_head->as_CountedLoop();
+  if (cl->is_normal_loop()) return;
   Node *cl_exit = cl->loopexit_or_null();
   int rpo_idx = _post_block.length();


And I saw [JDK-8275330](https://github.com/openjdk/jdk/pull/6429) by Roland fixed the same assertion failure before. And the test case which causes the failure looks similar with this one - there is an inner dead loop which fails to be optimized away by C2.

Hi @rwestrel , may I ask if you have any ideas or hints about this?

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

PR: https://git.openjdk.org/jdk19/pull/130