On Tue, Sep 4, 2012 at 11:56 AM, Bin Cheng <bin.ch...@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.ch...@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).
Richard.
> Thanks very much.
>
>
>
