On Tue, Sep 4, 2012 at 4:33 PM, Richard Earnshaw <rearn...@arm.com> wrote: > On 04/09/12 15:31, Richard Guenther wrote: >> On Tue, Sep 4, 2012 at 12:53 PM, Richard Earnshaw <rearn...@arm.com> wrote: >>> On 04/09/12 11:11, Richard Guenther wrote: >>>> 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). >>>> >>> >>> 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. >>>>> >>>>> >>>>> >>>> >>> >>> >>> >>> >> > > > >
