Re: [PATCH] Simple improvement for predicate computation in if-convert phase.

[Richard Biener <richard.guenther@gmail.com>]

On Fri, Oct 17, 2014 at 3:08 PM, Yuri Rumyantsev <ysrumyan@gmail.com> wrote:
> Jeff,
>
> I prepared another patch that includes test-case as you requested.
>
> Below are answers on your questions.
>
>> First, for the benefit of anyone trying to understand what you're doing, defining what "cd equivalent" means would be >helpful.
>
> I added the following  comment to function:
>
>    fwe call basic blocks bb1 and bb2
>    cd-equivalent if they are executed under the same condition.
>
>
> Is it sufficient?
>
>>So, do you have a case where the dominated_by_p test above is true and is_predicated(bb) returns true as well?  I think this >part of the change is largely responsible for the hack you're doing with having the function scoped static variable join_bb.
>
> I don't have such test-case and I assume that if bb is always
> executed, it is not predicated.
>
> I also deleted "join_bb" in my changes.
>
>
> Is it OK for trunk now.

Ok.

Thanks,
Richard.

> Thanks.
> Yuri.
>
> 2014-10-17  Yuri Rumyantsev  <ysrumyan@gmail.com>
> gcc/ChangeLog
>
> * tree-if-conv.c (add_to_predicate_list): Check unconditionally
> that bb is always executed to early exit. Use predicate of
> cd-equivalent block for join blocks if it exists.
> (if_convertible_loop_p_1): Recompute POST_DOMINATOR tree.
> (tree_if_conversion): Free post-dominance information.
>
> gcc/testsuite/ChangeLog
>
> * gcc/dg/tree-ssa/ifc-cd.c: New test.
>
>
>
> 2014-10-17 1:16 GMT+04:00 Jeff Law <law@redhat.com>:
>> On 10/16/14 05:52, Yuri Rumyantsev wrote:
>>>
>>> Hi All,
>>>
>>> Here is a simple enhancement for predicate computation in if-convert
>>> phase:
>>>
>>>   We use notion of cd equivalence to get simpler predicate for
>>>       join block, e.g. if join block has 2 predecessors with predicates
>>>       p1 & p2 and p1 & !p2, we'd like to get p1 for it instead of
>>>       p1 & p2 | p1 & !p2.
>>>
>>> Bootstrap and regression testing did not show any new failures.
>>>
>>> Is it OK for trunk?
>>>
>>> gcc/ChangeLog
>>> 2014-10-16  Yuri Rumyantsev<ysrumyan@gmail.com>
>>>
>>> * tree-if-conv.c (add_to_predicate_list): Check unconditionally
>>> that bb is always executed to early exit. Use predicate of
>>> cd-equivalent block for join blocks if it exists.
>>> (if_convertible_loop_p_1): Recompute POST_DOMINATOR tree.
>>> (tree_if_conversion): Free post-dominance information.
>>
>> First, for the benefit of anyone trying to understand what you're doing,
>> defining what "cd equivalent" means would be helpful.
>>
>>
>>>
>>>
>>> if-conv.patch
>>>
>>>
>>> Index: tree-if-conv.c
>>> ===================================================================
>>> --- tree-if-conv.c      (revision 216217)
>>> +++ tree-if-conv.c      (working copy)
>>> @@ -396,25 +396,51 @@
>>>   }
>>>
>>>   /* Add condition NC to the predicate list of basic block BB.  LOOP is
>>> -   the loop to be if-converted.  */
>>> +   the loop to be if-converted. Use predicate of cd-equivalent block
>>> +   for join bb if it exists.  */
>>>
>>>   static inline void
>>>   add_to_predicate_list (struct loop *loop, basic_block bb, tree nc)
>>>   {
>>>     tree bc, *tp;
>>> +  basic_block dom_bb;
>>> +  static basic_block join_bb = NULL;
>>>
>>>     if (is_true_predicate (nc))
>>>       return;
>>>
>>> -  if (!is_predicated (bb))
>>> +  /* If dominance tells us this basic block is always executed,
>>> +     don't record any predicates for it.  */
>>> +  if (dominated_by_p (CDI_DOMINATORS, loop->latch, bb))
>>> +    return;
>>
>> So, do you have a case where the dominated_by_p test above is true and
>> is_predicated(bb) returns true as well?  I think this part of the change is
>> largely responsible for the hack you're doing with having the function
>> scoped static variable join_bb.
>>
>>
>>
>>
>>> +
>>> +  /* If predicate has been already set up for given bb using
>>> cd-equivalent
>>> +     block predicate, simply escape.  */
>>> +  if (join_bb == bb)
>>> +    return;
>>
>> I *really* dislike the state you're carrying around via join_bb.
>>
>> ISTM that if you compute that there's an equivalence, then you just set the
>> predicate for the equivalent block and the right things would have happened
>> if you had not changed the test above.
>>
>> You also need a testcase.  It doesn't have to be extensive, but at least
>> some basic "smoke test" to verify basic operation of this code.  It's
>> perfectly fine to scan the debugging dumps for debug output.
>>
>>
>> jeff
>>
>>