Unnecessary check on phi node in tree if-conversion?

[Bin.Cheng]

On Thu, Apr 7, 2016 at 10:30 AM, Richard Biener
<richard.guenther@gmail.com> wrote:
> 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.
We do need to handle aforementioned case though.  I suppose an
additional pass of dce before if-conv will fix the problem, or some
light weight functions can?

Thanks,
bin