On Thu, May 19, 2011 at 3:36 PM, Kai Tietz <ktiet...@googlemail.com> wrote:
> 2011/5/19 Richard Guenther <richard.guent...@gmail.com>:
>> On Thu, May 19, 2011 at 3:30 PM, Kai Tietz <ktiet...@googlemail.com> wrote:
>>> 2011/5/19 Richard Guenther <richard.guent...@gmail.com>:
>>>> On Thu, May 19, 2011 at 3:08 PM, Kai Tietz <ktiet...@googlemail.com> wrote:
>>>>> 2011/5/19 Richard Guenther <richard.guent...@gmail.com>:
>>>>>> On Thu, May 19, 2011 at 2:59 PM, Kai Tietz <ktiet...@googlemail.com>
>>>>>> wrote:
>>>>>>> 2011/5/19 Richard Guenther <richard.guent...@gmail.com>:
>>>>>>>> On Thu, May 19, 2011 at 2:48 PM, Kai Tietz <ktiet...@googlemail.com>
>>>>>>>> wrote:
>>>>>>>>> Hello,
>>>>>>>>>
>>>>>>>>> This patch improves reassociation folding for comparision. It expands
>>>>>>>>> expressions within binary-AND/OR expression like (X | Y) == 0 to (X ==
>>>>>>>>> 0 && Y == 0)
>>>>>>>>> and (X | Y) != 0 to (X != 0 || Y != 0). This is necessary to allow
>>>>>>>>> better reassociation
>>>>>>>>> on weak pre-folded logical expressions. This unfolding gets undone
>>>>>>>>> anyway later by pass,
>>>>>>>>> so no disadvantage gets introduced.
>>>>>>>>> Also while going through BB-list, it tries to do some little
>>>>>>>>> type-sinking for SSA sequences
>>>>>>>>> like "D1 = (type) bool1; D2 = (type) bool2; D3 = D1 & D2;' to 'D1 =
>>>>>>>>> bool1 & bool2; D2 = (type) D1;'.
>>>>>>>>> This folding has the advantage to see better through intermediate
>>>>>>>>> results with none-boolean type.
>>>>>>>>> The function eliminate_redundant_comparison () got reworded so, that
>>>>>>>>> doesn't break in all cases.
>>>>>>>>> It now continues to find duplicates and tries to find inverse variant
>>>>>>>>> (folded to constant). By this
>>>>>>>>> change we don't combine possible weak optimizations too fast, before
>>>>>>>>> we can find and handle
>>>>>>>>> inverse or duplicates.
>>>>>>>>
>>>>>>>> sinking casting belongs not here but instead to tree-ssa-forwprop.
>>>>>>>> I'm not sure that a != 0 | b != 0 is the better canonical variant than
>>>>>>>> a | b != 0 though.
>>>>>>>>
>>>>>>>> is_boolean_compatible_type_p looks like a strange remanent.
>>>>>>>>
>>>>>>>> Richard.
>>>>>>>
>>>>>>> Well, a | b != 0 is for sure more optimal, but for reassociation we
>>>>>>> need to see the unfolded variant temporary. This is necessary as
>>>>>>> fold-const can't see through SSA statements. But this kind of
>>>>>>> expansion should be reversed then by pass to the form (a | b) != 0
>>>>>>> back.
>>>>>>
>>>>>> ? fold-const shouldn't deal with this at all as we are in gimple and in
>>>>>> SSA form. Surely re-association comes to play only with chains of
>>>>>> the above with more than two operands.
>>>>>>
>>>>>> Richard.
>>>>>>
>>>>>>>
>>>>>>> Regards,
>>>>>>> Kai
>>>>>>>
>>>>>>
>>>>>
>>>>> The issue you can see by testcase binop_tor4.c, as here are the
>>>>> intermediate variables d and e (with int type) are destroying the
>>>>> reassociation pass. This testcase for example needs this sinking.
>>>>
>>>> hoisting would work equally well
>>>
>>> Well, but just if then all operands in combined BIT_AND/OR block are
>>> getting hoisting. And well, there might be still some cases where we
>>> wouldn't find the equivalent. As hoisting leads to following
>>> sequences, eg:
>>>
>>> D1 = a != 0;
>>> D2 = b != 0;
>>> D3 = a == 0;
>>> D4 = b == 0;
>>> D5 = (long) D1
>>> D6 = (long) D2
>>> D7 = (long) D3
>>> D8 = (long) D4
>>> D9 = D5 & D6;
>>> D10 = D8 & D9
>>> D11 = D9 & D10;
>>>
>>> which means that comparision folding will never will happen as the
>>> statement passed to fold algorithm is a cast to a comparison and not
>>> the comparison itself. So sinking looks more sane IMHO.
>>
>> The above is what you do.
>
> No, I don't do this. Please see function sink_cast_and_expand function in
> patch.
>
> if (gimple_assign_cast_p (s1)
> && gimple_assign_cast_p (s2)
> && is_boolean_compatible_type_p (TREE_TYPE (gimple_assign_rhs1 (s1)))
> && is_boolean_compatible_type_p (TREE_TYPE (gimple_assign_rhs1 (s2)))
> && useless_type_conversion_p (TREE_TYPE (gimple_assign_rhs1 (s1)),
> TREE_TYPE (gimple_assign_rhs1 (s2))))
> {
> gimple_stmt_iterator gsi;
> gimple sum;
> tree op1a, op1b, tmpvar;
>
> tmpvar = create_tmp_reg (TREE_TYPE (gimple_assign_rhs1 (s1)), NULL);
>
> add_referenced_var (tmpvar);
> sum = build_and_add_sum (tmpvar, gimple_assign_rhs1 (s1),
> gimple_assign_rhs1 (s2), code);
> op1 = gimple_get_lhs (sum);
> op1 = fold_convert (type, op1);
>
> op1a = gimple_assign_rhs1 (stmt);
> op1b = gimple_assign_rhs2 (stmt);
> gsi = gsi_for_stmt (stmt);
> gimple_assign_set_rhs_from_tree (&gsi, op1);
> update_stmt (stmt);
> remove_visited_stmt_chain (op1a);
> remove_visited_stmt_chain (op1b);
> ret = true;
> }
>
> The none-boolean cast get moved outer, not inner.
You said (X | Y) != 0 to (X != 0 || Y != 0). I would simply move everything
down, including the cast (that's already done by tree-ssa-forwprop.c).
Richard.
> Regards,
> Kai
>
