* [PATCH][ARM][cleanup] Use IN_RANGE more often
@ 2015-04-15 15:22 Kyrill Tkachov
2015-04-18 14:18 ` Richard Earnshaw
0 siblings, 1 reply; 5+ messages in thread
From: Kyrill Tkachov @ 2015-04-15 15:22 UTC (permalink / raw)
To: GCC Patches; +Cc: Ramana Radhakrishnan, Richard Earnshaw
[-- Attachment #1: Type: text/plain, Size: 2274 bytes --]
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?
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.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: arm-in-range.patch --]
[-- Type: text/x-patch; name=arm-in-range.patch, Size: 19944 bytes --]
commit 1a11ec5c32789211bc778d010773b1a415914127
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date: Thu Mar 19 11:57:40 2015 +0000
[ARM] Use IN_RANGE more often
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 21409a0..54102f6 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -5333,14 +5333,14 @@ arm_return_in_memory (const_tree type, const_tree fntype)
memory (unless they're over 16 bytes, which will break since
we only have four call-clobbered registers to play with). */
if (TREE_CODE (type) == VECTOR_TYPE)
- return (size < 0 || size > (4 * UNITS_PER_WORD));
+ return !IN_RANGE (size, 0, 4 * UNITS_PER_WORD);
/* The rest go in memory. */
return true;
}
if (TREE_CODE (type) == VECTOR_TYPE)
- return (size < 0 || size > (4 * UNITS_PER_WORD));
+ return !IN_RANGE (size, 0, 4 * UNITS_PER_WORD);
if (!AGGREGATE_TYPE_P (type) &&
(TREE_CODE (type) != VECTOR_TYPE))
@@ -5351,7 +5351,7 @@ arm_return_in_memory (const_tree type, const_tree fntype)
{
/* ATPCS and later return aggregate types in memory only if they are
larger than a word (or are variable size). */
- return (size < 0 || size > UNITS_PER_WORD);
+ return !IN_RANGE (size, 0, UNITS_PER_WORD);
}
/* For the arm-wince targets we choose to be compatible with Microsoft's
@@ -5361,7 +5361,7 @@ arm_return_in_memory (const_tree type, const_tree fntype)
Also catch the case where int_size_in_bytes returns -1. In this case
the aggregate is either huge or of variable size, and in either case
we will want to return it via memory and not in a register. */
- if (size < 0 || size > UNITS_PER_WORD)
+ if (!IN_RANGE (size, 0, UNITS_PER_WORD))
return true;
if (TREE_CODE (type) == RECORD_TYPE)
@@ -5757,7 +5757,7 @@ aapcs_vfp_is_call_or_return_candidate (enum arm_pcs pcs_variant,
{
int ag_count = aapcs_vfp_sub_candidate (type, &new_mode);
- if (ag_count > 0 && ag_count <= 4)
+ if (IN_RANGE (ag_count, 1, 4))
*count = ag_count;
else
return false;
@@ -6984,8 +6984,7 @@ thumb_find_work_register (unsigned long pushed_regs_mask)
when a function has an unused argument in r3. But it is better to be
safe than to be sorry. */
if (! cfun->machine->uses_anonymous_args
- && crtl->args.size >= 0
- && crtl->args.size <= (LAST_ARG_REGNUM * UNITS_PER_WORD)
+ && IN_RANGE (crtl->args.size, 0, LAST_ARG_REGNUM * UNITS_PER_WORD)
&& (TARGET_AAPCS_BASED
? crtl->args.info.aapcs_ncrn < 4
: crtl->args.info.nregs < 4))
@@ -7273,9 +7272,9 @@ thumb2_legitimate_address_p (machine_mode mode, rtx x, int strict_p)
offset = INTVAL(addend);
if (GET_MODE_SIZE (mode) <= 4)
- return (offset > -256 && offset < 256);
+ return IN_RANGE (offset, -255, 255);
- return (use_ldrd && offset > -1024 && offset < 1024
+ return (use_ldrd && IN_RANGE (offset, -1023, 1023)
&& (offset & 3) == 0);
}
@@ -7347,8 +7346,7 @@ arm_legitimate_index_p (machine_mode mode, rtx index, RTX_CODE outer,
if (TARGET_HARD_FLOAT
&& TARGET_VFP
&& (mode == SFmode || mode == DFmode))
- return (code == CONST_INT && INTVAL (index) < 1024
- && INTVAL (index) > -1024
+ return (code == CONST_INT && IN_RANGE (INTVAL (index), -1023, 1023)
&& (INTVAL (index) & 3) == 0);
/* For quad modes, we restrict the constant offset to be slightly less
@@ -7358,22 +7356,19 @@ arm_legitimate_index_p (machine_mode mode, rtx index, RTX_CODE outer,
(double-mode) offset and so should INDEX+8. */
if (TARGET_NEON && VALID_NEON_QREG_MODE (mode))
return (code == CONST_INT
- && INTVAL (index) < 1016
- && INTVAL (index) > -1024
+ && IN_RANGE (INTVAL (index), -1023, 1015)
&& (INTVAL (index) & 3) == 0);
/* We have no such constraint on double mode offsets, so we permit the
full range of the instruction format. */
if (TARGET_NEON && VALID_NEON_DREG_MODE (mode))
return (code == CONST_INT
- && INTVAL (index) < 1024
- && INTVAL (index) > -1024
+ && IN_RANGE (INTVAL (index), -1023, 1023)
&& (INTVAL (index) & 3) == 0);
if (TARGET_REALLY_IWMMXT && VALID_IWMMXT_REG_MODE (mode))
return (code == CONST_INT
- && INTVAL (index) < 1024
- && INTVAL (index) > -1024
+ && IN_RANGE (INTVAL (index), -1023, 1023)
&& (INTVAL (index) & 3) == 0);
if (arm_address_register_rtx_p (index, strict_p)
@@ -7387,9 +7382,9 @@ arm_legitimate_index_p (machine_mode mode, rtx index, RTX_CODE outer,
HOST_WIDE_INT val = INTVAL (index);
if (TARGET_LDRD)
- return val > -256 && val < 256;
+ return IN_RANGE (val, -255, 255);
else
- return val > -4096 && val < 4092;
+ return IN_RANGE (val, -4095, 4091);
}
return TARGET_LDRD && arm_address_register_rtx_p (index, strict_p);
@@ -7467,12 +7462,12 @@ thumb2_legitimate_index_p (machine_mode mode, rtx index, int strict_p)
if (TARGET_HARD_FLOAT
&& TARGET_VFP
&& (mode == SFmode || mode == DFmode))
- return (code == CONST_INT && INTVAL (index) < 1024
+ return (code == CONST_INT
/* Thumb-2 allows only > -256 index range for it's core register
load/stores. Since we allow SF/DF in core registers, we have
to use the intersection between -256~4096 (core) and -1024~1024
(coprocessor). */
- && INTVAL (index) > -256
+ && IN_RANGE (INTVAL (index), -255, 1023)
&& (INTVAL (index) & 3) == 0);
if (TARGET_REALLY_IWMMXT && VALID_IWMMXT_REG_MODE (mode))
@@ -7481,8 +7476,7 @@ thumb2_legitimate_index_p (machine_mode mode, rtx index, int strict_p)
and only allow LDRD addressing modes. */
if (!TARGET_LDRD || mode != DImode)
return (code == CONST_INT
- && INTVAL (index) < 1024
- && INTVAL (index) > -1024
+ && IN_RANGE (INTVAL (index), -1023, 1023)
&& (INTVAL (index) & 3) == 0);
}
@@ -7493,16 +7487,14 @@ thumb2_legitimate_index_p (machine_mode mode, rtx index, int strict_p)
(double-mode) offset and so should INDEX+8. */
if (TARGET_NEON && VALID_NEON_QREG_MODE (mode))
return (code == CONST_INT
- && INTVAL (index) < 1016
- && INTVAL (index) > -1024
+ && IN_RANGE (INTVAL (index), -1023, 1015)
&& (INTVAL (index) & 3) == 0);
/* We have no such constraint on double mode offsets, so we permit the
full range of the instruction format. */
if (TARGET_NEON && VALID_NEON_DREG_MODE (mode))
return (code == CONST_INT
- && INTVAL (index) < 1024
- && INTVAL (index) > -1024
+ && IN_RANGE (INTVAL (index), -1023, 1023)
&& (INTVAL (index) & 3) == 0);
if (arm_address_register_rtx_p (index, strict_p)
@@ -7518,7 +7510,7 @@ thumb2_legitimate_index_p (machine_mode mode, rtx index, int strict_p)
/* Thumb-2 ldrd only has reg+const addressing modes. */
/* ldrd supports offsets of +-1020.
However the ldr fallback does not. */
- return val > -256 && val < 256 && (val & 3) == 0;
+ return IN_RANGE (val, -255, 255) && (val & 3) == 0;
}
else
return 0;
@@ -7540,13 +7532,11 @@ thumb2_legitimate_index_p (machine_mode mode, rtx index, int strict_p)
return (arm_address_register_rtx_p (XEXP (index, 0), strict_p)
&& CONST_INT_P (op)
- && INTVAL (op) > 0
- && INTVAL (op) <= 3);
+ && IN_RANGE (INTVAL (op), 1, 3));
}
return (code == CONST_INT
- && INTVAL (index) < 4096
- && INTVAL (index) > -256);
+ && IN_RANGE (INTVAL (index), -255, 4095));
}
/* Return nonzero if X is valid as a 16-bit Thumb state base register. */
@@ -7703,10 +7693,10 @@ thumb_legitimate_offset_p (machine_mode mode, HOST_WIDE_INT val)
switch (GET_MODE_SIZE (mode))
{
case 1:
- return val >= 0 && val < 32;
+ return IN_RANGE (val, 0, 31);
case 2:
- return val >= 0 && val < 64 && (val & 1) == 0;
+ return IN_RANGE (val, 0, 63) && (val & 1) == 0;
default:
return (val >= 0
@@ -8114,8 +8104,8 @@ thumb_legitimize_address (rtx x, rtx orig_x, machine_mode mode)
/* Try and fold the offset into a biasing of the base register and
then offsetting that. Don't do this when optimizing for space
since it can cause too many CSEs. */
- if (optimize_size && offset >= 0
- && offset < 256 + 31 * GET_MODE_SIZE (mode))
+ if (optimize_size
+ && IN_RANGE (offset, 0, 255 + 31 * GET_MODE_SIZE (mode)))
{
HOST_WIDE_INT delta;
@@ -8130,7 +8120,7 @@ thumb_legitimize_address (rtx x, rtx orig_x, machine_mode mode)
NULL_RTX);
x = plus_constant (Pmode, xop0, delta);
}
- else if (offset < 0 && offset > -256)
+ else if (IN_RANGE (offset, -255, -1))
/* Small negative offsets are best done with a subtract before the
dereference, forcing these into a register normally takes two
instructions. */
@@ -12476,7 +12466,7 @@ vfp3_const_double_index (rtx x)
if (mantissa == 0)
return -1;
- gcc_assert (mantissa >= 16 && mantissa <= 31);
+ gcc_assert (IN_RANGE (mantissa, 16, 31));
/* The value of 5 here would be 4 if GCC used IEEE754-like encoding (where
normalized significands are in the range [1, 2). (Our mantissa is shifted
@@ -12485,7 +12475,7 @@ vfp3_const_double_index (rtx x)
REAL_EXP must be altered. */
exponent = 5 - exponent;
- if (exponent < 0 || exponent > 7)
+ if (!IN_RANGE (exponent, 0, 7))
return -1;
/* Sign, mantissa and exponent are now in the correct form to plug into the
@@ -12792,7 +12782,7 @@ neon_immediate_valid_for_logic (rtx op, machine_mode mode, int inverse,
int tmpwidth;
int retval = neon_valid_immediate (op, mode, inverse, &tmpconst, &tmpwidth);
- if (retval < 0 || retval > 5)
+ if (!IN_RANGE (retval, 0, 5))
return 0;
if (modconst)
@@ -13143,7 +13133,7 @@ bounds_check (rtx operand, HOST_WIDE_INT low, HOST_WIDE_INT high,
lane = INTVAL (operand);
- if (lane < low || lane >= high)
+ if (!IN_RANGE (lane, low, high - 1))
error (err);
}
@@ -13954,7 +13944,7 @@ load_multiple_sequence (rtx *operands, int nops, int *regs, int *saved_order,
/* Can only handle up to MAX_LDM_STM_OPS insns at present, though could be
easily extended if required. */
- gcc_assert (nops >= 2 && nops <= MAX_LDM_STM_OPS);
+ gcc_assert (IN_RANGE (nops, 2, MAX_LDM_STM_OPS));
memset (order, 0, MAX_LDM_STM_OPS * sizeof (int));
@@ -14113,7 +14103,7 @@ store_multiple_sequence (rtx *operands, int nops, int nops_total,
/* Can only handle up to MAX_LDM_STM_OPS insns at present, though could be
easily extended if required. */
- gcc_assert (nops >= 2 && nops <= MAX_LDM_STM_OPS);
+ gcc_assert (IN_RANGE (nops, 2, MAX_LDM_STM_OPS));
memset (order, 0, MAX_LDM_STM_OPS * sizeof (int));
@@ -15977,7 +15967,7 @@ offset_ok_for_ldrd_strd (HOST_WIDE_INT offset)
else
return false;
- return ((offset <= max_offset) && (offset >= -max_offset));
+ return IN_RANGE (offset, -max_offset, max_offset);
}
/* Checks whether the operands are valid for use in an LDRD/STRD instruction.
@@ -19341,7 +19331,7 @@ output_ascii_pseudo_op (FILE *stream, const unsigned char *p, int len)
#define callee_saved_reg_p(reg) \
(!call_used_regs[reg] \
|| (TARGET_THUMB1 && optimize_size \
- && reg >= FIRST_HI_REGNUM && reg <= LAST_HI_REGNUM))
+ && IN_RANGE (reg, FIRST_HI_REGNUM, LAST_HI_REGNUM)))
/* Compute the register save mask for registers 0 through 12
inclusive. This code is used by arm_compute_save_reg_mask. */
@@ -20052,7 +20042,7 @@ thumb2_emit_strd_push (unsigned long saved_regs_mask)
num_regs = bit_count (saved_regs_mask);
/* Must be at least one register to save, and can't save SP or PC. */
- gcc_assert (num_regs > 0 && num_regs <= 14);
+ gcc_assert (IN_RANGE (num_regs, 1, 14));
gcc_assert (!(saved_regs_mask & (1 << SP_REGNUM)));
gcc_assert (!(saved_regs_mask & (1 << PC_REGNUM)));
@@ -26239,7 +26229,7 @@ arm_output_load_gr (rtx *operands)
|| GET_CODE (sum = XEXP (operands [1], 0)) != PLUS
|| !REG_P (reg = XEXP (sum, 0))
|| !CONST_INT_P (offset = XEXP (sum, 1))
- || ((INTVAL (offset) < 1024) && (INTVAL (offset) > -1024)))
+ || (IN_RANGE (INTVAL (offset), -1023, 1023)))
return "wldrw%?\t%0, %1";
/* Fix up an out-of-range load of a GR register. */
@@ -26556,7 +26546,7 @@ arm_array_mode_supported_p (machine_mode mode,
{
if (TARGET_NEON
&& (VALID_NEON_DREG_MODE (mode) || VALID_NEON_QREG_MODE (mode))
- && (nelems >= 2 && nelems <= 4))
+ && IN_RANGE (nelems, 2, 4))
return true;
return false;
diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
index 8c10ea3..7526ae7 100644
--- a/gcc/config/arm/arm.h
+++ b/gcc/config/arm/arm.h
@@ -1008,9 +1008,10 @@ extern int arm_arch_crc;
#define LAST_IWMMXT_GR_REGNUM (FIRST_IWMMXT_GR_REGNUM + 3)
#define IS_IWMMXT_REGNUM(REGNUM) \
- (((REGNUM) >= FIRST_IWMMXT_REGNUM) && ((REGNUM) <= LAST_IWMMXT_REGNUM))
+ (IN_RANGE ((REGNUM), FIRST_IWMMXT_REGNUM, LAST_IWMMXT_REGNUM))
+
#define IS_IWMMXT_GR_REGNUM(REGNUM) \
- (((REGNUM) >= FIRST_IWMMXT_GR_REGNUM) && ((REGNUM) <= LAST_IWMMXT_GR_REGNUM))
+ (IN_RANGE ((REGNUM), FIRST_IWMMXT_GR_REGNUM, LAST_IWMMXT_GR_REGNUM))
/* Base register for access to local variables of the function. */
#define FRAME_POINTER_REGNUM 102
@@ -1024,7 +1025,7 @@ extern int arm_arch_crc;
(TARGET_VFPD32 ? LAST_HI_VFP_REGNUM : LAST_LO_VFP_REGNUM)
#define IS_VFP_REGNUM(REGNUM) \
- (((REGNUM) >= FIRST_VFP_REGNUM) && ((REGNUM) <= LAST_VFP_REGNUM))
+ (IN_RANGE ((REGNUM), FIRST_VFP_REGNUM, LAST_VFP_REGNUM))
/* VFP registers are split into two types: those defined by VFP versions < 3
have D registers overlaid on consecutive pairs of S registers. VFP version 3
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index dcc4446..ac94404 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -2310,7 +2310,7 @@ (define_insn "*zeroextractsi_compare0_scratch"
(match_operand 2 "const_int_operand" "n"))
(const_int 0)))]
"TARGET_32BIT
- && (INTVAL (operands[2]) >= 0 && INTVAL (operands[2]) < 32
+ && (IN_RANGE (INTVAL (operands[2]), 0, 31)
&& INTVAL (operands[1]) > 0
&& INTVAL (operands[1]) + (INTVAL (operands[2]) & 1) <= 8
&& INTVAL (operands[1]) + INTVAL (operands[2]) <= 32)"
@@ -2335,13 +2335,13 @@ (define_insn_and_split "*ne_zeroextractsi"
(const_int 0)))
(clobber (reg:CC CC_REGNUM))]
"TARGET_32BIT
- && (INTVAL (operands[3]) >= 0 && INTVAL (operands[3]) < 32
+ && (IN_RANGE (INTVAL (operands[3]), 0, 31)
&& INTVAL (operands[2]) > 0
&& INTVAL (operands[2]) + (INTVAL (operands[3]) & 1) <= 8
&& INTVAL (operands[2]) + INTVAL (operands[3]) <= 32)"
"#"
"TARGET_32BIT
- && (INTVAL (operands[3]) >= 0 && INTVAL (operands[3]) < 32
+ && (IN_RANGE (INTVAL (operands[3]), 0, 31)
&& INTVAL (operands[2]) > 0
&& INTVAL (operands[2]) + (INTVAL (operands[3]) & 1) <= 8
&& INTVAL (operands[2]) + INTVAL (operands[3]) <= 32)"
@@ -2401,14 +2401,14 @@ (define_insn_and_split "*ite_ne_zeroextractsi"
(const_int 0)))
(clobber (reg:CC CC_REGNUM))]
"TARGET_ARM
- && (INTVAL (operands[3]) >= 0 && INTVAL (operands[3]) < 32
+ && (IN_RANGE (INTVAL (operands[3]), 0, 31)
&& INTVAL (operands[2]) > 0
&& INTVAL (operands[2]) + (INTVAL (operands[3]) & 1) <= 8
&& INTVAL (operands[2]) + INTVAL (operands[3]) <= 32)
&& !reg_overlap_mentioned_p (operands[0], operands[4])"
"#"
"TARGET_ARM
- && (INTVAL (operands[3]) >= 0 && INTVAL (operands[3]) < 32
+ && (IN_RANGE (INTVAL (operands[3]), 0, 31)
&& INTVAL (operands[2]) > 0
&& INTVAL (operands[2]) + (INTVAL (operands[3]) & 1) <= 8
&& INTVAL (operands[2]) + INTVAL (operands[3]) <= 32)
@@ -6775,8 +6775,7 @@ (define_expand "load_multiple"
/* Support only fixed point registers. */
if (!CONST_INT_P (operands[2])
- || INTVAL (operands[2]) > MAX_LDM_STM_OPS
- || INTVAL (operands[2]) < 2
+ || !IN_RANGE (INTVAL (operands[2]), 2, MAX_LDM_STM_OPS)
|| !MEM_P (operands[1])
|| !REG_P (operands[0])
|| REGNO (operands[0]) > (LAST_ARM_REGNUM - 1)
@@ -6800,8 +6799,7 @@ (define_expand "store_multiple"
/* Support only fixed point registers. */
if (!CONST_INT_P (operands[2])
- || INTVAL (operands[2]) > MAX_LDM_STM_OPS
- || INTVAL (operands[2]) < 2
+ || !IN_RANGE (INTVAL (operands[2]), 2, MAX_LDM_STM_OPS)
|| !REG_P (operands[1])
|| !MEM_P (operands[0])
|| REGNO (operands[1]) > (LAST_ARM_REGNUM - 1)
diff --git a/gcc/config/arm/neon.md b/gcc/config/arm/neon.md
index 445df2a..e40f8f4 100644
--- a/gcc/config/arm/neon.md
+++ b/gcc/config/arm/neon.md
@@ -1007,7 +1007,7 @@ (define_insn "ashldi3_neon_noclobber"
(match_operand:DI 2 "reg_or_int_operand" " i,w")))]
"TARGET_NEON && reload_completed
&& (!CONST_INT_P (operands[2])
- || (INTVAL (operands[2]) >= 0 && INTVAL (operands[2]) < 64))"
+ || IN_RANGE (INTVAL (operands[2]), 0, 63))"
"@
vshl.u64\t%P0, %P1, %2
vshl.u64\t%P0, %P1, %P2"
@@ -1095,7 +1095,7 @@ (define_insn "ashrdi3_neon_imm_noclobber"
(ashiftrt:DI (match_operand:DI 1 "s_register_operand" " w")
(match_operand:DI 2 "const_int_operand" " i")))]
"TARGET_NEON && reload_completed
- && INTVAL (operands[2]) > 0 && INTVAL (operands[2]) <= 64"
+ && IN_RANGE (INTVAL (operands[2]), 1, 64)"
"vshr.s64\t%P0, %P1, %2"
[(set_attr "type" "neon_shift_imm")]
)
@@ -1105,7 +1105,7 @@ (define_insn "lshrdi3_neon_imm_noclobber"
(lshiftrt:DI (match_operand:DI 1 "s_register_operand" " w")
(match_operand:DI 2 "const_int_operand" " i")))]
"TARGET_NEON && reload_completed
- && INTVAL (operands[2]) > 0 && INTVAL (operands[2]) <= 64"
+ && IN_RANGE (INTVAL (operands[2]), 1, 64)"
"vshr.u64\t%P0, %P1, %2"
[(set_attr "type" "neon_shift_imm")]
)
diff --git a/gcc/config/arm/thumb1.md b/gcc/config/arm/thumb1.md
index 3847b33..5d1bf4c 100644
--- a/gcc/config/arm/thumb1.md
+++ b/gcc/config/arm/thumb1.md
@@ -82,7 +82,7 @@ (define_insn_and_split "*thumb1_addsi3"
"
"&& reload_completed && CONST_INT_P (operands[2])
&& ((operands[1] != stack_pointer_rtx
- && (INTVAL (operands[2]) > 255 || INTVAL (operands[2]) < -255))
+ && !IN_RANGE (INTVAL (operands[2]), -255, 255))
|| (operands[1] == stack_pointer_rtx
&& INTVAL (operands[2]) > 1020))"
[(set (match_dup 0) (plus:SI (match_dup 1) (match_dup 2)))
diff --git a/gcc/config/arm/thumb2.md b/gcc/config/arm/thumb2.md
index 1f68147..2a6b087 100644
--- a/gcc/config/arm/thumb2.md
+++ b/gcc/config/arm/thumb2.md
@@ -1517,7 +1517,7 @@ (define_peephole2
(match_operand 5 "" "")
(match_operand 6 "" "")))]
"TARGET_THUMB2
- && (INTVAL (operands[2]) >= 0 && INTVAL (operands[2]) < 32)"
+ && (IN_RANGE (INTVAL (operands[2]), 0, 31))"
[(parallel [(set (match_dup 0)
(compare:CC_NOOV (ashift:SI (match_dup 1) (match_dup 2))
(const_int 0)))
@@ -1545,7 +1545,7 @@ (define_peephole2
(match_operand 5 "" "")
(match_operand 6 "" "")))]
"TARGET_THUMB2
- && (INTVAL (operands[2]) > 0 && INTVAL (operands[2]) < 32)"
+ && IN_RANGE (INTVAL (operands[2]), 1, 31)"
[(parallel [(set (match_dup 0)
(compare:CC_NOOV (ashift:SI (match_dup 1) (match_dup 2))
(const_int 0)))
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH][ARM][cleanup] Use IN_RANGE more often
2015-04-15 15:22 [PATCH][ARM][cleanup] Use IN_RANGE more often Kyrill Tkachov
@ 2015-04-18 14:18 ` Richard Earnshaw
2015-04-20 11:03 ` Kyrill Tkachov
0 siblings, 1 reply; 5+ messages in thread
From: Richard Earnshaw @ 2015-04-18 14:18 UTC (permalink / raw)
To: Kyrill Tkachov, GCC Patches; +Cc: Ramana Radhakrishnan, Richard Earnshaw
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.
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).
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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH][ARM][cleanup] Use IN_RANGE more often
2015-04-18 14:18 ` Richard Earnshaw
@ 2015-04-20 11:03 ` Kyrill Tkachov
2015-04-20 11:07 ` Jakub Jelinek
2015-04-20 11:31 ` Richard Earnshaw
0 siblings, 2 replies; 5+ messages in thread
From: Kyrill Tkachov @ 2015-04-20 11:03 UTC (permalink / raw)
To: Richard Earnshaw, GCC Patches; +Cc: Ramana Radhakrishnan, Richard Earnshaw
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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH][ARM][cleanup] Use IN_RANGE more often
2015-04-20 11:03 ` Kyrill Tkachov
@ 2015-04-20 11:07 ` Jakub Jelinek
2015-04-20 11:31 ` Richard Earnshaw
1 sibling, 0 replies; 5+ messages in thread
From: Jakub Jelinek @ 2015-04-20 11:07 UTC (permalink / raw)
To: Kyrill Tkachov
Cc: Richard Earnshaw, GCC Patches, Ramana Radhakrishnan, Richard Earnshaw
On Mon, Apr 20, 2015 at 12:03:11PM +0100, Kyrill Tkachov wrote:
> 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?
Of course it does, as long as the types of VALUE, LOWER and UPPER aren't
wider integer types than {,un}signed HOST_WIDE_INT. As HWI is always 64-bit
now, that just means it wouldn't work for 128-bit integral types that aren't
used anywhere in GCC. So, just don't use IN_RANGE for floating point
values, that might have surprising effects, otherwise it is safe to use it.
Jakub
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH][ARM][cleanup] Use IN_RANGE more often
2015-04-20 11:03 ` Kyrill Tkachov
2015-04-20 11:07 ` Jakub Jelinek
@ 2015-04-20 11:31 ` Richard Earnshaw
1 sibling, 0 replies; 5+ messages in thread
From: Richard Earnshaw @ 2015-04-20 11:31 UTC (permalink / raw)
To: Kyrill Tkachov, GCC Patches; +Cc: Ramana Radhakrishnan, Richard Earnshaw
On 20/04/15 12:03, Kyrill Tkachov wrote:
>
> 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.
>
I suggest we just drop the bits that are not using HWI as the base type.
R.
> 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.
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-04-20 11:31 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-15 15:22 [PATCH][ARM][cleanup] Use IN_RANGE more often Kyrill Tkachov
2015-04-18 14:18 ` Richard Earnshaw
2015-04-20 11:03 ` Kyrill Tkachov
2015-04-20 11:07 ` Jakub Jelinek
2015-04-20 11:31 ` Richard Earnshaw
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).