Re: [patch tree-optimization]: Improve handling of conditional-branches on targets with high branch costs

[Kai Tietz <ktietz70 at googlemail dot com>]

2011/10/11 Michael Matz <matz@suse.de>:
> Hi,
>
> On Mon, 10 Oct 2011, Kai Tietz wrote:
>
>> To ensure that we use simple_operand_p in all cases, beside for
>> branching AND/OR chains, in same way as before, I added to this function
>> an additional argument, by which the looking into comparisons can be
>> activated.
>
> Better make it a separate function the first tests your new conditions,
> and then calls simple_operand_p.

Well, either I make it a new function and call it instead of
simple_operand_p, or I need to modify old simple_operand_p.  The logic
of new and old version is incompatible and has not to be called on
same tree, as otherwise new check is vain.

>> +fold_truth_andor_1 (location_t loc, enum tree_code code, tree truth_type,
>> + ? ? ? ? ? ? ? ? tree lhs, tree rhs)
>> ?{
>> ? ?/* If this is the "or" of two comparisons, we can do something if
>> ? ? ? the comparisons are NE_EXPR. ?If this is the "and", we can do something
>> @@ -5149,13 +5176,6 @@ fold_truthop (location_t loc, enum tree_
>> ? ? ? ? ? ? ? ? ? ? ? ? ?build2 (BIT_IOR_EXPR, TREE_TYPE (ll_arg),
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?ll_arg, rl_arg),
>> ? ? ? ? ? ? ? ? ? ? ? ? ?build_int_cst (TREE_TYPE (ll_arg), 0));
>> -
>> - ? ? ?if (LOGICAL_OP_NON_SHORT_CIRCUIT)
>> - ? ? {
>> - ? ? ? if (code != orig_code || lhs != orig_lhs || rhs != orig_rhs)
>> - ? ? ? ? return build2_loc (loc, code, truth_type, lhs, rhs);
>> - ? ? ? return NULL_TREE;
>> - ? ? }
>
> Why do you remove this hunk? ?Shouldn't you instead move the hunk you
> added to fold_truth_andor() here. ?I realize this needs some TLC to
> fold_truth_andor_1, because right now it early-outs for non-comparisons,
> but it seems the better place. ?I.e. somehow move the below code into the
> above branch, with the associated diddling on fold_truth_andor_1 that it
> gets called.

This hunk is removed, as it is vain to do here.  Btw richi asked for
it, and I agree that new TRUTH-AND/OR packing is better done at a
single place in fold_truth_andor only.

>> + ?if ((code == TRUTH_ANDIF_EXPR || code == TRUTH_ORIF_EXPR)
>> + ? ? ?&& (BRANCH_COST (optimize_function_for_speed_p (cfun),
>> + ? ? ? ? ? ? ? ? ? ?false) >= 2)
>> + ? ? ?&& !TREE_SIDE_EFFECTS (arg1)
>> + ? ? ?&& LOGICAL_OP_NON_SHORT_CIRCUIT
>> + ? ? ?&& simple_operand_p (arg1, true))
>> + ? ?{
>> + ? ? ?enum tree_code ncode = (code == TRUTH_ANDIF_EXPR ? TRUTH_AND_EXPR
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?: TRUTH_OR_EXPR);
>> +
>> + ? ? ?/* We don't want to pack more then two leafs to an non-IF
>
> Missing continuation of the sentence?

Well, here is a colon missing.

>> + ? ? ? ? If tree-code of left-hand operand isn't an AND/OR-IF code and not
>> + ? ? ? ? equal to CODE, then we don't want to add right-hand operand.
>> + ? ? ? ? If the inner right-hand side of left-hand operand has side-effects,
>> + ? ? ? ? or isn't simple, then we can't add to it, as otherwise we might
>> + ? ? ? ? destroy if-sequence. ?*/
>
>
>> + ? ? ?if (TREE_CODE (arg0) == code
>> + ? ? ? ? ? ? ? /* Needed for sequence points to handle trappings, and
>> + ? ? ? ? ? ? ? ? ?side-effects. ?*/
>> + ? ? ? ? ? ? ? && !TREE_SIDE_EFFECTS (TREE_OPERAND (arg0, 1))
>> + ? ? ? ? ? ? ? && simple_operand_p (TREE_OPERAND (arg0, 1), true))
>> + ? ? ? {
>> + ? ? ? ? tem = fold_build2_loc (loc, ncode, type, TREE_OPERAND (arg0, 1),
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? arg1);
>> + ? ? ? ? return fold_build2_loc (loc, code, type, TREE_OPERAND (arg0, 0),
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?tem);
>> + ? ? ? }
>> + ? ? /* Needed for sequence points to handle trappings, and side-effects. ?*/
>> + ? ? else if (!TREE_SIDE_EFFECTS (arg0)
>> + ? ? ? ? ? && simple_operand_p (arg0, true))
>> + ? ? ? return fold_build2_loc (loc, ncode, type, arg0, arg1);
>> + ? ?}
>> +
>
>
> Ciao,
> Michael.
>

Kai