This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH][ARM][cleanup] Use IN_RANGE more often



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 >= lo && a <= 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 >= 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) \
   <= (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-linux
system. This seems like a not trivial increase to me. If that is not acceptable
then we can drop this patch.

Thanks,
Kyrill


R.

Thanks,
Kyrill

2015-04-15  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     * 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_si):
     Likewise.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]