From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 123628 invoked by alias); 20 Apr 2015 11:03:27 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 123610 invoked by uid 89); 20 Apr 2015 11:03:25 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.5 required=5.0 tests=AWL,BAYES_50,SPF_PASS autolearn=ham version=3.3.2 X-HELO: eu-smtp-delivery-143.mimecast.com Received: from eu-smtp-delivery-143.mimecast.com (HELO eu-smtp-delivery-143.mimecast.com) (207.82.80.143) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 20 Apr 2015 11:03:15 +0000 Received: from cam-owa1.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.140]) by uk-mta-22.uk.mimecast.lan; Mon, 20 Apr 2015 12:03:12 +0100 Received: from [10.2.207.50] ([10.1.2.79]) by cam-owa1.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Mon, 20 Apr 2015 12:03:12 +0100 Message-ID: <5534DCEF.6000305@arm.com> Date: Mon, 20 Apr 2015 11:03:00 -0000 From: Kyrill Tkachov User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: Richard Earnshaw , GCC Patches CC: Ramana Radhakrishnan , Richard Earnshaw Subject: Re: [PATCH][ARM][cleanup] Use IN_RANGE more often References: <552E8221.2080401@arm.com> <553267C3.2030509@foss.arm.com> In-Reply-To: <553267C3.2030509@foss.arm.com> X-MC-Unique: BqXOL6xsRXK10cmvcnGfXQ-1 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2015-04/txt/msg01010.txt.bz2 On 18/04/15 15:18, Richard Earnshaw wrote: > On 15/04/15 16:22, Kyrill Tkachov wrote: >> Hi all, >> >> This patch goes through the arm backend and replaces expressions of the >> form >> a >=3D lo && a <=3D hi with IN_RANGE (a, lo, hi) which is that tiny bit = smaller >> and easier to read in my opinion. I guess there's also a chance it might >> make >> things infinitesimally faster since IN_RANGE evaluates 'a' only once. >> The patch also substitutes expressions like a > hi || a < lo with >> !IN_RANGE (a, lo, hi) which, again, conveys the intended meaning more >> clearly. >> I tried to make sure not to introduce any off-by-one errors and testing >> caught some that I had made while writing these. >> >> Bootstrapped and tested arm-none-linux-gnueabihf. Built and run SPEC2006 >> succesfully. >> >> Ok for trunk once 5.1 is released? >> > I think this is pretty obvious for those cases where the type of the > range is [unsigned] HOST_WIDE_INT, but much less obvious for those cases > where the type is just int, or unsigned. Cases that I think need more > careful examination include vfp3_const_double_index and > aapcs_vfp_is_call_or_return_candidate, but I haven't gone through every > instance to check whether there are more cases. The definition and comment on IN_RANGE in system.h is: /* A macro to determine whether a VALUE lies inclusively within a certain range without evaluating the VALUE more than once. This macro won't warn if the VALUE is unsigned and the LOWER bound is zero, as it would e.g. with "VALUE >=3D 0 && ...". Note the LOWER bound *is* evaluated twice, and LOWER must not be greater than UPPER. However the bounds themselves can be either positive or negative. */ #define IN_RANGE(VALUE, LOWER, UPPER) \ ((unsigned HOST_WIDE_INT) (VALUE) - (unsigned HOST_WIDE_INT) (LOWER) \ <=3D (unsigned HOST_WIDE_INT) (UPPER) - (unsigned HOST_WIDE_INT) (LOWER= )) Since it works on positive or negative bounds, I'd think it would work on signed numbers, wouldn't it? > > I'd be particularly concerned about these if the widening of the result > caused a code quality regression on a native 32-bit machine (since HWI > is a 64-bit type). That being said, I see a 0.6% size increase on cc1 built on a native arm-li= nux system. This seems like a not trivial increase to me. If that is not accept= able then we can drop this patch. Thanks, Kyrill > > R. > >> Thanks, >> Kyrill >> >> 2015-04-15 Kyrylo Tkachov >> >> * config/arm/arm.md (*zeroextractsi_compare0_scratch): Use IN_RANGE >> instead of two compares. >> (*ne_zeroextractsi): Likewise. >> (*ite_ne_zeroextractsi): Likewise. >> (load_multiple): Likewise. >> (store_multiple): Likewise. >> * config/arm/arm.h (IS_IWMMXT_REGNUM): Likewise. >> (IS_IWMMXT_GR_REGNUM): Likewise. >> (IS_VFP_REGNUM): Likewise. >> * config/arm/arm.c (arm_return_in_memory): Likewise. >> (aapcs_vfp_is_call_or_return_candidate): Likewise. >> (thumb_find_work_register): Likewise. >> (thumb2_legitimate_address_p): Likewise. >> (arm_legitimate_index_p): Likewise. >> (thumb2_legitimate_index_p): Likewise. >> (thumb1_legitimate_address_p): Likewise. >> (thumb_legitimize_address): Likewise. >> (vfp3_const_double_index): Likewise. >> (neon_immediate_valid_for_logic): Likewise. >> (bounds_check): Likewise. >> (load_multiple_sequence): Likewise. >> (store_multiple_sequence): Likewise. >> (offset_ok_for_ldrd_strd): Likewise. >> (callee_saved_reg_p): Likewise. >> (thumb2_emit_strd_push): Likewise. >> (arm_output_load_gr): Likewise. >> (arm_vector_mode_supported_p): Likewise. >> * config/arm/neon.md (ashldi3_neon_noclobber): Likewise. >> (ashrdi3_neon_imm_noclobber): Likewise. >> (lshrdi3_neon_imm_noclobber): Likewise. >> * config/arm/thumb1.md (*thumb1_addsi3): Likewise. >> * config/arm/thumb2.md (define_peephole2's after orsi_not_shiftsi_s= i): >> Likewise.