Re: Unnecessary check on phi node in tree if-conversion?

[Richard Biener <richard dot guenther at gmail dot com>]

On April 6, 2016 8:21:35 PM GMT+02:00, "Bin.Cheng" <amker.cheng@gmail.com> wrote:
>On Wed, Apr 6, 2016 at 5:07 PM, Bin.Cheng <amker.cheng@gmail.com>
>wrote:
>> Hi,
>> Function if_convertible_phi_p has below check on virtual PHI nodes:
>>
>>
>>   if (any_mask_load_store)
>>     return true;
>>
>>   /* When there were no if-convertible stores, check
>>      that there are no memory writes in the branches of the loop to
>be
>>      if-converted.  */
>>   if (virtual_operand_p (gimple_phi_result (phi)))
>>     {
>>       imm_use_iterator imm_iter;
>>       use_operand_p use_p;
>>
>>       if (bb != loop->header)
>>     {
>>       if (dump_file && (dump_flags & TDF_DETAILS))
>>         fprintf (dump_file, "Virtual phi not on loop->header.\n");
>>       return false;
>>     }
>>
>>       FOR_EACH_IMM_USE_FAST (use_p, imm_iter, gimple_phi_result
>(phi))
>>     {
>>       if (gimple_code (USE_STMT (use_p)) == GIMPLE_PHI
>>           && USE_STMT (use_p) != phi)
>>         {
>>           if (dump_file && (dump_flags & TDF_DETAILS))
>>         fprintf (dump_file, "Difficult to handle this virtual
>phi.\n");
>>           return false;
>>         }
>>     }
>>     }
>>
>> Since the check is short-cut by any_mask_load_store, when the check
>is
>> triggered, it means there is virtual phi node but no conditional
>> memory stores?  Is this possible?  Plus, any_mask_load_store is set
>by
>> below code in if_convertible_gimple_assign_stmt_p:
>>       if (ifcvt_can_use_mask_load_store (stmt))
>>     {
>>       gimple_set_plf (stmt, GF_PLF_2, true);
>>       *any_mask_load_store = true;
>>       return true;
>>     }
>>
>> So in theory it's possible to skip aforementioned check when only
>mask
>> load is encountered.
>>
>> Any ideas?
>It's possible to have a loop like:
>
><bb header>
>  .MEM_2232 = PHI <.MEM_574(179), .MEM_1247(183)>
>  ...
>  if (cond)
>    goto <bb 1>
>  else
>    goto <bb2>
>
><bb 1>:  //empty
><bb2>:
>  .MEM_1247 = PHI <.MEM_2232(180), .MEM_2232(181)>
>  if (cond2)
>    goto <bb exit>
>  else
>    goto <bb latch>
>
><bb latch>:
>  goto <bb header>
>
>So can we handle the PHI which can be degenerated in if-cvt?

I believe the check is bogus and we should drop it.

Richard.

>Thanks,
>bin