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

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

On April 8, 2016 4:55:59 PM GMT+02:00, "Bin.Cheng" <amker.cheng@gmail.com> wrote:
>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?

I don't see any issue with the above?

Richard.

>Thanks,
>bin