This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On Tue, Sep 4, 2012 at 4:33 PM, Richard Earnshaw <rearnsha@arm.com> wrote:
> On 04/09/12 15:31, Richard Guenther wrote:
>> On Tue, Sep 4, 2012 at 12:53 PM, Richard Earnshaw <rearnsha@arm.com> wrote:
>>> 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.
>>
>> Then you define LOGICAL_OP_NON_SHORT_CIRCUIT to zero.  The above
>> would be an identity transform for all targets currently, so "it is not working
>> for modern ARM cores" anyway.
>>
>
> No, that's backwards.  That gives us branches around compares, not
> formation of or'ed cflag values that we can then transform into
> conditional compares.

I see.  So I suppose for that reason the original patch is ok.

Thanks,
Richard.

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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]