Hi Dominik,
On 16/11/16 12:16, Dominik Vogt wrote:
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.)
are you sure? IN_RANGE is supposed to avoid such warnings.
Its definition 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))
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?
Bootstrap succeeded with this patch, and I didn't see any warnings.
In the definition of IN_RANGE given above there is no direct comparison (>=)
with zero
so there's nothing to warn above.
Thanks,
Kyrill
+ && 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 ^_^ ^_^