2011/5/19 Richard Guenther <[email protected]>:
> On Thu, May 19, 2011 at 3:36 PM, Kai Tietz <[email protected]> wrote:
>> 2011/5/19 Richard Guenther <[email protected]>:
>>> On Thu, May 19, 2011 at 3:30 PM, Kai Tietz <[email protected]> wrote:
>>>> 2011/5/19 Richard Guenther <[email protected]>:
>>>>> On Thu, May 19, 2011 at 3:08 PM, Kai Tietz <[email protected]>
>>>>> wrote:
>>>>>> 2011/5/19 Richard Guenther <[email protected]>:
>>>>>>> On Thu, May 19, 2011 at 2:59 PM, Kai Tietz <[email protected]>
>>>>>>> wrote:
>>>>>>>> 2011/5/19 Richard Guenther <[email protected]>:
>>>>>>>>> On Thu, May 19, 2011 at 2:48 PM, Kai Tietz <[email protected]>
>>>>>>>>> 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).
Yes, here I do also already sinking. See build_expand_ne_eq_cmp
function (which is called by sink_cast_and_expand () - tree is here
unrolled recursively.
if (is_boolean_compatible_type_p (type))
op1 = fold_build2 ((code == NE_EXPR ? BIT_IOR_EXPR : BIT_AND_EXPR),
type, op1a, op1b);
else
{
gimple sum;
tree tmpvar = create_tmp_reg (boolean_type_node, NULL);
add_referenced_var (tmpvar);
sum = build_and_add_sum (tmpvar, op1a, op1b,
(code == NE_EXPR ? BIT_IOR_EXPR
: BIT_AND_EXPR));
op1 = gimple_get_lhs (sum);
op1 = fold_convert (type, op1);
}
For internal expansions of (A | B | C) != to the series of A!=0 & B !=
0 & C != 0 will done only on boolean_type_node type. There is no
recast. Just on final expansion it gets recasted.
So yes, the way is to move everything down as far as possible. I don't
see your point here that I wouldn't "simply move everything down" by
this patch. If I wouldn't the testcases would fail.
Regards,
Kai
