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

[Yuri Rumyantsev <ysrumyan at gmail dot com>]

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.

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
>
>

Attachment: if-conv.patch.new
Description: Binary data