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