This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH][ARM] PR target/67929 Tighten vfp3_const_double_for_bits checks
- From: Kyrill Tkachov <kyrylo dot tkachov at arm dot com>
- To: GCC Patches <gcc-patches at gcc dot gnu dot org>
- Cc: Ramana Radhakrishnan <ramana dot radhakrishnan at arm dot com>, Richard Earnshaw <Richard dot Earnshaw at arm dot com>
- Date: Wed, 14 Oct 2015 13:23:29 +0100
- Subject: [PATCH][ARM] PR target/67929 Tighten vfp3_const_double_for_bits checks
- Authentication-results: sourceware.org; auth=none
Hi all,
This patch fixes the referenced PR by rewriting the vfp3_const_double_for_bits function in arm.c
The function is supposed to accept positive CONST_DOUBLE rtxes whose value is an exact power of 2
and whose log2 is between 1 and 32. That is values like 2.0, 4.0, 8.9, 16.0 etc...
The current implementation seems to have been written under the assumption that exact_real_truncate returns
false if the input value is not an exact integer, whereas in fact exact_real_truncate returns false if the
truncation operation was not exact, which are different things. This would lead the function to accept any
CONST_DOUBLE that can truncate to a power of 2, such as 4.9, 16.2 etc.
In any case, I've rewritten this function and used the real_isinteger predicate to check if the real value
is an exact integer.
The testcase demonstrates the kind of wrong code that this patch addresses.
This bug appears on GCC 5 and 4.9 as well, but due to the recent introduction of CONST_DOUBLE_REAL_VALUE
this patch doesn't apply on those branches. I will soon post the backportable variant.
Bootstrapped and tested on arm-none-linux-gnueabihf.
Ok for trunk?
Thanks,
Kyrill
2015-10-12 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
PR target/67929
* config/arm/arm.c (vfp3_const_double_for_bits): Rewrite.
* config/arm/constraints.md (Dp): Update callsite.
* config/arm/predicates.md (const_double_vcvt_power_of_two): Likewise.
2015-10-12 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
PR target/67929
* gcc.target/arm/pr67929_1.c: New test.
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 0bf1164..29dd489 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -27734,25 +27734,37 @@ vfp3_const_double_for_fract_bits (rtx operand)
return 0;
}
+/* If X is a CONST_DOUBLE with a value that is a power of 2 whose
+ log2 is in [1, 32], return that log2. Otherwise return -1.
+ This is used in the patterns for vcvt.s32.f32 floating-point to
+ fixed-point conversions. */
+
int
-vfp3_const_double_for_bits (rtx operand)
+vfp3_const_double_for_bits (rtx x)
{
- const REAL_VALUE_TYPE *r0;
+ const REAL_VALUE_TYPE *r;
- if (!CONST_DOUBLE_P (operand))
- return 0;
+ if (!CONST_DOUBLE_P (x))
+ return -1;
- r0 = CONST_DOUBLE_REAL_VALUE (operand);
- if (exact_real_truncate (DFmode, r0))
- {
- HOST_WIDE_INT value = real_to_integer (r0);
- value = value & 0xffffffff;
- if ((value != 0) && ( (value & (value - 1)) == 0))
- return int_log2 (value);
- }
+ r = CONST_DOUBLE_REAL_VALUE (x);
- return 0;
+ if (REAL_VALUE_NEGATIVE (*r)
+ || REAL_VALUE_ISNAN (*r)
+ || REAL_VALUE_ISINF (*r)
+ || !real_isinteger (r, SFmode))
+ return -1;
+
+ HOST_WIDE_INT hwint = exact_log2 (real_to_integer (r));
+
+/* The exact_log2 above will have returned -1 if this is
+ not an exact log2. */
+ if (!IN_RANGE (hwint, 1, 32))
+ return -1;
+
+ return hwint;
}
+
/* Emit a memory barrier around an atomic sequence according to MODEL. */
diff --git a/gcc/config/arm/constraints.md b/gcc/config/arm/constraints.md
index e24858f..901cfe5 100644
--- a/gcc/config/arm/constraints.md
+++ b/gcc/config/arm/constraints.md
@@ -339,7 +339,8 @@
"@internal
In ARM/ Thumb2 a const_double which can be used with a vcvt.s32.f32 with bits operation"
(and (match_code "const_double")
- (match_test "TARGET_32BIT && TARGET_VFP && vfp3_const_double_for_bits (op)")))
+ (match_test "TARGET_32BIT && TARGET_VFP
+ && vfp3_const_double_for_bits (op) > 0")))
(define_register_constraint "Ts" "(arm_restrict_it) ? LO_REGS : GENERAL_REGS"
"For arm_restrict_it the core registers @code{r0}-@code{r7}. GENERAL_REGS otherwise.")
diff --git a/gcc/config/arm/predicates.md b/gcc/config/arm/predicates.md
index 08cc899..48e4ba8 100644
--- a/gcc/config/arm/predicates.md
+++ b/gcc/config/arm/predicates.md
@@ -668,7 +668,7 @@
(define_predicate "const_double_vcvt_power_of_two"
(and (match_code "const_double")
(match_test "TARGET_32BIT && TARGET_VFP
- && vfp3_const_double_for_bits (op)")))
+ && vfp3_const_double_for_bits (op) > 0")))
(define_predicate "neon_struct_operand"
(and (match_code "mem")
diff --git a/gcc/testsuite/gcc.target/arm/pr67929_1.c b/gcc/testsuite/gcc.target/arm/pr67929_1.c
new file mode 100644
index 0000000..14943b6
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr67929_1.c
@@ -0,0 +1,21 @@
+/* { dg-do run } */
+/* { dg-require-effective-target arm_vfp3_ok } */
+/* { dg-options "-O2 -fno-inline" } */
+/* { dg-add-options arm_vfp3 } */
+/* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */
+
+int
+foo (float a)
+{
+ return a * 4.9f;
+}
+
+
+int
+main (void)
+{
+ if (foo (10.0f) != 49)
+ __builtin_abort ();
+
+ return 0;
+}
\ No newline at end of file