On Wed, Nov 16, 2016 at 08:45:35AM +0000, Kyrill Tkachov wrote:
> This patch fixes the arm build failure due to out of range ubfx operands.
> Combine now more aggressively generates zero_extracts
> and it's up to the backend to reject invalid bit offsets and widths. And arm
> seems to suffer from the same problems as aarch64 and s390
> did in PR 77822.
>
> My ARMv7-A and ARMv7-R Architecture Reference Manual version C.c in section
> A8.8.246 says that the bit offset (<lsb>) should be
> in the range 0 to 31 whereas the width should be in the range 1 to 32 -
> <lsb>. Same for SBFX.
>
> This patch directly translates those restrictions into range checks on
> operands 2 and 3 of the relevant patterns.
>
> With this patch the arm build succeeds.
> Bootstrapped and tested on arm-none-linux-gnueabihf.
>
> Committing to trunk in the interest of fixing the build.
Should this check be put into a common code macro? I'm just
working on a trunk patch for s390[x] and have this in s390.h.
#define S390_EXTRACT_ARGS_IN_RANGE(SIZE, POS, MODE) \
((unsigned HOST_WIDE_INT)(POS) < GET_MODE_BITSIZE (MODE) \
&& (SIZE) > 0 && (SIZE) <= GET_MODE_BITSIZE (MODE) \
&& IN_RANGE ((SIZE) + (POS), 1, GET_MODE_BITSIZE (MODE)))
(I couldn't use IN_RANGE to test the value range of SIZE because it
generates an "always true" warning when the lower bound is zero.)
This should be the correct test for any target that cannot handle
"out of range" arguments to {sign,zero}_extract.
> 2016-11-16 Kyrylo Tkachov <kyrylo.tkac...@arm.com>
>
> PR target/78364
> * config/arm/arm.md (*extv_reg): Restrict operands 2 and 3 to the
> proper ranges for an SBFX instruction.
> (extzv_t2): Likewise for UBFX.
> commit b9ea5a6274834cbc469988040a807093156b52cf
> Author: Kyrylo Tkachov <kyrylo.tkac...@arm.com>
> Date: Tue Nov 15 15:53:21 2016 +0000
>
> [ARM] PR target/78364: Add proper restrictions to zero and sign_extract
> patterns operands
>
> diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
> index ac3ef15..87b5ea6 100644
> --- a/gcc/config/arm/arm.md
> +++ b/gcc/config/arm/arm.md
> @@ -4561,9 +4561,11 @@ (define_insn "unaligned_storehi"
> (define_insn "*extv_reg"
> [(set (match_operand:SI 0 "s_register_operand" "=r")
> (sign_extract:SI (match_operand:SI 1 "s_register_operand" "r")
> - (match_operand:SI 2 "const_int_M_operand" "M")
> - (match_operand:SI 3 "const_int_M_operand" "M")))]
> - "arm_arch_thumb2"
> + (match_operand:SI 2 "const_int_operand" "n")
> + (match_operand:SI 3 "const_int_operand" "n")))]
> + "arm_arch_thumb2
> + && IN_RANGE (INTVAL (operands[3]), 0, 31)
Doesn't this generate a warning for you?
> + && IN_RANGE (INTVAL (operands[2]), 1, 32 - INTVAL (operands[3]))"
> "sbfx%?\t%0, %1, %3, %2"
> [(set_attr "length" "4")
> (set_attr "predicable" "yes")
> @@ -4574,9 +4576,11 @@ (define_insn "*extv_reg"
> (define_insn "extzv_t2"
> [(set (match_operand:SI 0 "s_register_operand" "=r")
> (zero_extract:SI (match_operand:SI 1 "s_register_operand" "r")
> - (match_operand:SI 2 "const_int_M_operand" "M")
> - (match_operand:SI 3 "const_int_M_operand" "M")))]
> - "arm_arch_thumb2"
> + (match_operand:SI 2 "const_int_operand" "n")
> + (match_operand:SI 3 "const_int_operand" "n")))]
> + "arm_arch_thumb2
> + && IN_RANGE (INTVAL (operands[3]), 0, 31)
> + && IN_RANGE (INTVAL (operands[2]), 1, 32 - INTVAL (operands[3]))"
> "ubfx%?\t%0, %1, %3, %2"
> [(set_attr "length" "4")
> (set_attr "predicable" "yes")
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt
IBM Germany
