2011/10/6 Richard Guenther <richard.guent...@gmail.com>:
> On Thu, Oct 6, 2011 at 11:28 AM, Kai Tietz <kti...@redhat.com> wrote:
>> Hello,
>>
>> Sorry attached non-updated change. Here with proper attached patch.
>> This patch improves in fold_truth_andor the generation of branch-conditions
>> for targets having LOGICAL_OP_NON_SHORT_CIRCUIT set. If right-hand side
>> operation of a TRUTH_(OR|AND)IF_EXPR is simple operand, has no side-effects,
>> and doesn't trap, then try to convert expression to a TRUTH_(AND|OR)_EXPR,
>> if left-hand operand is a simple operand, and has no side-effects.
>>
>> ChangeLog
>>
>> 2011-10-06 Kai Tietz <kti...@redhat.com>
>>
>> * fold-const.c (fold_truth_andor): Convert TRUTH_(AND|OR)IF_EXPR
>> to TRUTH_OR_EXPR, if suitable.
>>
>> Bootstrapped and tested for all languages (including Ada and Obj-C++) on
>> host x86_64-unknown-linux-gnu. Ok for apply?
>>
>> Regards,
>> Kai
>>
>>
>> ndex: fold-const.c
>> ===================================================================
>> --- fold-const.c (revision 179592)
>> +++ fold-const.c (working copy)
>> @@ -8387,6 +8387,33 @@
>> if ((tem = fold_truthop (loc, code, type, arg0, arg1)) != 0)
>> return tem;
>>
>> + if ((code == TRUTH_ANDIF_EXPR || code == TRUTH_ORIF_EXPR)
>> + && !TREE_SIDE_EFFECTS (arg1)
>> + && simple_operand_p (arg1)
>> + && LOGICAL_OP_NON_SHORT_CIRCUIT
>
> Why only for LOGICAL_OP_NON_SHORT_CIRCUIT? It doesn't make
> a difference for !LOGICAL_OP_NON_SHORT_CIRCUIT targets, but ...
Well, I used this check only for not doing this transformation for
targets, which have low-cost branches. This is the same thing as in
fold_truthop. It does this transformation only if
LOGICAL_OP_NON_SHORT_CIRCUIT is true.
>> + && !FLOAT_TYPE_P (TREE_TYPE (arg1))
>
> ? I hope we don't have &&|| float.
This can happen. Operands of TRUTH_AND|OR(IF)_EXPR aren't necessarily
of integral type. After expansion in gimplifier, we have for sure
comparisons, but not in c-tree.
>> + && ((TREE_CODE_CLASS (TREE_CODE (arg1)) != tcc_comparison
>> + && TREE_CODE (arg1) != TRUTH_NOT_EXPR)
>> + || !FLOAT_TYPE_P (TREE_TYPE (TREE_OPERAND (arg1, 0)))))
>
> ? simple_operand_p would have rejected both ! and comparisons.
This check is the same as in fold_truthop. I used this check. The
point here is that floats might trap.
> I miss a test for side-effects on arg0 (and probably simple_operand_p there,
> as well).
See inner of if condition for those checks. I moved those checks for
arg1 out of the inner conditions to avoid double-checking.
>> + {
>> + if (TREE_CODE (arg0) == code
>> + && !TREE_SIDE_EFFECTS (TREE_OPERAND (arg0, 1))
>> + && simple_operand_p (TREE_OPERAND (arg0, 1)))
>
> Err ... so why do you recurse here (and associate)? Even with different
> predicates than above ...
See, here is the missing check. Point is that even if arg0 has
side-effects and is a (AND|OR)IF expression, we might be able to
associate with right-hand argument of arg0, if for it no side-effects
are existing. Otherwise we wouldn't catch this case.
We have here in maximum a recursion level of one.
> And similar transforms seem to happen in fold_truthop - did you
> investigate why it didn't trigger there.
This is pretty simple. The point is that only for comparisons this
transformation is done. But in c-tree we don't have here necessarily
for TRUTH_(AND|OR)[IF]_EXPR comparison arguments, not necessarily
integral ones (see above).
> And I'm missing a testcase.
Ok, I'll add one. Effect can be seen best after gimplification.
> Richard.
>
>> + {
>> + tem = build2_loc (loc,
>> + (code == TRUTH_ANDIF_EXPR ? TRUTH_AND_EXPR
>> + : TRUTH_OR_EXPR),
>> + type, TREE_OPERAND (arg0, 1), arg1);
>> + return fold_build2_loc (loc, code, type, TREE_OPERAND (arg0, 0),
>> tem);
>> + }
>> + if (!TREE_SIDE_EFFECTS (arg0)
>> + && simple_operand_p (arg0))
>> + return build2_loc (loc,
>> + (code == TRUTH_ANDIF_EXPR ? TRUTH_AND_EXPR
>> + : TRUTH_OR_EXPR),
>> + type, arg0, arg1);
>> + }
>> +
>> return NULL_TREE;
>> }
Regards.
Kai
