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. Richard. > R. > >> Richard. >> >>> Thanks very much. >>> >>> >>> >> > > > >
