On 03/03/15 17:59, Kyrylo Tkachov wrote:
-----Original Message----- From: Kyrylo Tkachov Sent: 27 February 2015 14:30 To: Kyrylo Tkachov; GCC Patches Cc: Ramana Radhakrishnan; Richard Earnshaw Subject: RE: [PATCH][ARM] PR target/64600 Fix another ICE with - mtune=xscale: properly sign-extend mask during constant splitting On 03/02/15 15:18, Kyrill Tkachov wrote:Hi all, The ICE in this PR occurs when -mtune=xscale triggers a particular path through arm_gen_constant during expand that creates a 0xfffff00f mask but for a 64-bit HOST_WIDE_INT doesn't sign extend it into 0xfffffffffffff00f that signifies the required -4081. It leaves it as 0xfffff00f (4294963215) that breaks when later combine tries to perform an SImode bitwise AND using the wide-int machinery. I think the correct approach here is to use trunc_int_for_mode that correctly sign-extends the constant so that it is properly represented by a HOST_WIDE_INT for the required mode. Bootstrapped and tested arm-none-linux-gnueabihf with -mtune=xscale in BOOT_CFLAGS. The testcase triggers for -mcpu=xscale and all slowmul targets because they are the only ones that have the constant_limit tune parameter set to anything >1 which is required to follow this particular path through arm_split_constant. Also, the rtx costs can hide this ICE sometimes. Ok for trunk? Thanks, Kyrill 2015-02-03 Kyrylo Tkachov <kyrylo.tkac...@arm.com> PR target/64600 * config/arm/arm.c (arm_gen_constant, AND case): Call trunc_int_for_mode when constructing AND mask. 2015-02-03 Kyrylo Tkachov <kyrylo.tkac...@arm.com> PR target/64600 * gcc.target/arm/pr64600_1.c: New test. arm-xscale-wide.patch commit 52388a359dd65276bccfac499a2fd9e406fbe1a8 Author: Kyrylo Tkachov <kyrylo.tkac...@arm.com> Date: Tue Jan 20 11:21:34 2015 +0000 [ARM] Fix ICE due to arm_gen_constant not sign_extending diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index db4834b..d0f3a52 100644 --- a/gcc/config/arm/arm.c +++ b/gcc/config/arm/arm.c @@ -4709,19 +4709,20 @@ arm_gen_constant (enum rtx_code code, machine_mode mode, rtx cond, if ((remainder | shift_mask) != 0xffffffff) { + HOST_WIDE_INT new_val + = trunc_int_for_mode (remainder | shift_mask, mode);Offlist, Richard mentioned that trunc_int_for_mode may pessimize codegen for HImode values due to excessive setting of bits and using ARM_SIGN_EXTEND might be preferable. I've tried that and it does fix the ICE and goes through testing ok. Bootstrap still ongoing. I didn't perform any code quality investigation. Richard, are there any particular code sequences that you'd like us to investigate here?Here's the alternative version using ARM_SIGN_EXTEND if you want to have a look. Thanks, Kyrill 2015-03-03 Kyrylo Tkachov <kyrylo.tkac...@arm.com> PR target/64600 * config/arm/arm.c (arm_gen_constant, AND case): Use ARM_SIGN_EXTEND when constructing AND mask. 2015-03-03 Kyrylo Tkachov <kyrylo.tkac...@arm.com> PR target/64600 * gcc.target/arm/pr64600_1.c: New test.Thanks, Kyrill
This is OK. Let's take the ARM_SIGN_EXTEND version instead of the other one given what Richard says.
regards Ramana
+ if (generate) { rtx new_src = subtargets ? gen_reg_rtx (mode) : target; - insns = arm_gen_constant (AND, mode, cond, - remainder | shift_mask, + insns = arm_gen_constant (AND, SImode, cond, new_val, new_src, source, subtargets, 1); source = new_src; } else { rtx targ = subtargets ? NULL_RTX : target; - insns = arm_gen_constant (AND, mode, cond, - remainder | shift_mask, + insns = arm_gen_constant (AND, mode, cond, new_val, targ, source, subtargets, 0); } } @@ -4744,12 +4745,13 @@ arm_gen_constant (enum rtx_code code, machine_mode mode, rtx cond, if ((remainder | shift_mask) != 0xffffffff) { + HOST_WIDE_INT new_val + = trunc_int_for_mode (remainder | shift_mask, mode); if (generate) { rtx new_src = subtargets ? gen_reg_rtx (mode) : target; - insns = arm_gen_constant (AND, mode, cond, - remainder | shift_mask, + insns = arm_gen_constant (AND, mode, cond, new_val, new_src, source, subtargets, 1); source = new_src; } @@ -4757,8 +4759,7 @@ arm_gen_constant (enum rtx_code code,machine_mode mode, rtx cond,{ rtx targ = subtargets ? NULL_RTX : target; - insns = arm_gen_constant (AND, mode, cond, - remainder | shift_mask, + insns = arm_gen_constant (AND, mode, cond, new_val, targ, source, subtargets, 0); } } diff --git a/gcc/testsuite/gcc.target/arm/pr64600_1.c b/gcc/testsuite/gcc.target/arm/pr64600_1.c new file mode 100644 index 0000000..6ba3fa2 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pr64600_1.c @@ -0,0 +1,15 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -mtune=xscale" } */ + +typedef unsigned int speed_t; +typedef unsigned int tcflag_t; + +struct termios { + tcflag_t c_cflag; +}; + +speed_t +cfgetospeed (const struct termios *tp) { + return tp->c_cflag & 010017; +}
