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