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

[Richard Earnshaw <rearnsha at arm dot com>]

On 04/09/12 11:11, Richard Guenther wrote:
> On Tue, Sep 4, 2012 at 11:56 AM, Bin Cheng <bin.cheng@arm.com> wrote:
>>>> -----Original Message-----
>>>> From: Richard Earnshaw
>>>> Sent: Thursday, July 26, 2012 9:19 PM
>>>> To: Andrew Pinski
>>>> Cc: Bin Cheng; gcc-patches@gcc.gnu.org
>>>> Subject: Re: [PATCH]Remove duplicate check on BRANCH_COST in
>>>> fold-const.c
>>>>
>>>> On 26/07/12 11:27, Andrew Pinski wrote:
>>>>> On Thu, Jul 26, 2012 at 3:20 AM, Bin Cheng <bin.cheng@arm.com> wrote:
>>>>>> Hi,
>>>>>> This patch removes the duplicate check on BRANCH_COST in
>>> fold_truth_andor.
>>>>>> The BRANCH_COST condition removed is a duplicate of the default
>>>>>> definition of LOGICAL_OP_NON_SHORT_CIRCUIT.
>>>>>> All current targets (mips and rs6000) that provide non-default
>>>>>> definitions of LOGICAL_OP_SHORT_CIRCUIT set it to 0, so this patch
>>>>>> is therefore just a code cleanup and does not change behaviour in
>>>>>> the
>>> compiler.
>>>>>>
>>>>>> I built mipsel-elf cross compiler and compared newlib/libstdc++
>>>>>> compiled by the patched/original compilers.
>>>>>>
>>>>>> Is it OK?
>>>>>
>>>>> Just some history here on this.  The BRANCH COST check was there
>>>>> before LOGICAL_OP_NON_SHORT_CIRCUIT was added.  I will be submitting
>>>>> a patch which changes the MIPS definition soon but it will not be
>>>>> based on the branch cost but rather than another option.  So in the
>>>>> end it might not be redundant as it is currently.
>>>>>
>>>>> Thanks,
>>>>> Andrew
>>>>>
>>>>
>>>> You can always factor BRANCH_COST into LOGICAL_OP_NON_SHORT_CIRCUIT
>>>> (as
>>> the
>>>> default currently does), so there's no loss of functionality from
>>>> removing this currently redundant check.  However, the current
>>>> definition is broken
>>> in
>>>> that it makes it impossible to force the compiler to use this
>>>> optimization when the branch cost is low.
>>>>
>>>
>>
>> Hi, is this change ok? Or we need more discussion on it?
> 
> 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.

R.

> Richard.
> 
>> Thanks very much.
>>
>>
>>
>