Re: Ping^2: [PATCH]Remove duplicate check on BRANCH_COST in fold-const.c

["Bin.Cheng" <amker dot cheng at gmail dot com>]

Sorry, I mis-sent this offline.

On Tue, Sep 4, 2012 at 11:00 PM, Bin.Cheng <amker.cheng@gmail.com> wrote:
>>>
>>> It's not ok (I btw fail to see the patch in this thread).  The current
>>> way LOGICAL_OP_NON_SHORT_CIRCUIT is implemented/used should instead
>>> be changed to always match the pattern
>>>
>>>       LOGICAL_OP_NON_SHORT_CIRCUIT
>>>       && (BRANCH_COST (optimize_function_for_speed_p (cfun),
>>>                     false) >= 2)
>>>
>>> and the default value of LOGICAL_OP_NON_SHORT_CIRCUIT should be 1,
>>> defined in defaults.h (and the docs updated).
>>>
>>
>> That's not going to work for modern ARM cores.  We want to set
>> BRANCH_COST to 1 but still have it generate the non-short-circuit code
>> (because conditional compares are really cheap.
>>
>
> Hi Richard,
> For now, LOGICAL_OP_NON_SHORT_CIRCUIT macro is defined as below, which
> is duplicate of the BRANCH_COST condition.
>
> #ifndef LOGICAL_OP_NON_SHORT_CIRCUIT
> #define LOGICAL_OP_NON_SHORT_CIRCUIT \
>   (BRANCH_COST (optimize_function_for_speed_p (cfun), \
>                 false) >= 2)
> #endif
>
> Recently we measured performance on some ARM processors and found it
> would be better to have non-short-circuit optimization while setting
> BRANCH_COST to 1, which is impossible with present codes. So here
> comes this patch as below:
>
> Index: gcc/fold-const.c
> ===================================================================
> --- gcc/fold-const.c    (revision 189835)
> +++ gcc/fold-const.c    (working copy)
> @@ -8443,9 +8443,7 @@
>    if ((tem = fold_truth_andor_1 (loc, code, type, arg0, arg1)) != 0)
>      return tem;
>
> -  if ((BRANCH_COST (optimize_function_for_speed_p (cfun),
> -                   false) >= 2)
> -      && LOGICAL_OP_NON_SHORT_CIRCUIT
> +  if (LOGICAL_OP_NON_SHORT_CIRCUIT
>        && (code == TRUTH_AND_EXPR
>            || code == TRUTH_ANDIF_EXPR
>            || code == TRUTH_OR_EXPR
>
> The purpose is to remove the duplicate check on BRANCH_COST.
>
> As Andrew pointed out that the patch may change behavior if some
> back-ends define the macro independent of BRANCH_COST. After looking
> into the code, there are two uses of the macro in fold-const.c, each
> controls one kind code transformation. The first use is:
>
>   else if (LOGICAL_OP_NON_SHORT_CIRCUIT
>            && lhs != 0 && rhs != 0
>            && (code == TRUTH_ANDIF_EXPR
>                || code == TRUTH_ORIF_EXPR)
>            && operand_equal_p (lhs, rhs, 0))
>
> The second one is:
>
>   if ((BRANCH_COST (optimize_function_for_speed_p (cfun),
>                     false) >= 2)
>       && LOGICAL_OP_NON_SHORT_CIRCUIT
>       && (code == TRUTH_AND_EXPR
>           || code == TRUTH_ANDIF_EXPR
>           || code == TRUTH_OR_EXPR
>           || code == TRUTH_ORIF_EXPR))
>
> I am not sure why the 2nd condition is designed in current way and
> haven't found any useful changelog on it.
> But considering back end can factor BRANCH_COST in
> LOGICAL_OP_NON_SHORT_CIRCUIT or not, we can conclude that the behavior
> will only be changed if some back-end want to control the two
> transformations differently. So the problem becomes whether the 2nd
> condition should be changed. Either way there is scenario cannot be
> covered.
>

And for now,
FTR, only two targets redefine L_O_N_S_C: mips and rs6000.  Both set it
to zero so won't be affected by this change.


-- 
Best Regards.