[PATCH][AArch64] PR target/68129: Define TARGET_SUPPORTS_WIDE_INT

[PATCH][AArch64] PR target/68129: Define TARGET_SUPPORTS_WIDE_INT

[Kyrill Tkachov]

[-- Attachment #1: Type: text/plain, Size: 1733 bytes --]

Hi all,

In this PR we hit an ICE in simplify-rtx.c:
#if TARGET_SUPPORTS_WIDE_INT == 0
       /* This assert keeps the simplification from producing a result
      that cannot be represented in a CONST_DOUBLE but a lot of
      upstream callers expect that this function never fails to
      simplify something and so you if you added this to the test
      above the code would die later anyway.  If this assert
      happens, you just need to make the port support wide int.  */
       gcc_assert (width <= HOST_BITS_PER_DOUBLE_INT);
#endif

The aarch64 port does not define TARGET_SUPPORTS_WIDE_INT.
 From what I understand, in order to define it we need to remove the instances in the backend where
we use CONST_DOUBLE rtxes with VOIDmode (i.e. for storing CONST_INTs).
 From what I can see we don't create such instances, so this patch defines TARGET_SUPPORTS_WIDE_INT
and cleans up a couple of places in the backend that try to hypothetically handle CONST_DOUBLEs as CONST_INTs
if they are passed down as such from other places in the compiler.

Bootstrapped and tested on aarch64-none-linux-gnu on trunk and GCC 5.
No codegen differences on SPEC2006.

Ok for trunk and GCC 5?

Thanks,
Kyrill

2015-11-09  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     PR target/68129
     * config/aarch64/aarch64.h (TARGET_SUPPORTS_WIDE_INT): Define to 1.
     * config/aarch64/aarch64.c (aarch64_print_operand, CONST_DOUBLE):
     Delete VOIDmode case.  Assert that mode is not VOIDmode.
     * config/aarch64/predicates.md (const0_operand): Remove const_double
     match.

2015-11-09  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     PR target/68129
     * gcc.target/aarch64/pr68129_1.c: New test.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: aarch64-target-supports-wide-int.patch --]
[-- Type: text/x-patch; name=aarch64-target-supports-wide-int.patch, Size: 2531 bytes --]

commit 10562c44766a57e4762e926f876f5457f9899e33
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date:   Wed Oct 28 10:49:44 2015 +0000

    [AArch64] PR target/68129: Define TARGET_SUPPORTS_WIDE_INT

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 47ccc32..389bfc0 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -4401,11 +4401,10 @@ aarch64_print_operand (FILE *f, rtx x, char code)
 	  break;
 
 	case CONST_DOUBLE:
-	  /* CONST_DOUBLE can represent a double-width integer.
-	     In this case, the mode of x is VOIDmode.  */
-	  if (GET_MODE (x) == VOIDmode)
-	    ; /* Do Nothing.  */
-	  else if (aarch64_float_const_zero_rtx_p (x))
+	  /* Since we define TARGET_SUPPORTS_WIDE_INT we shouldn't ever
+	     be getting CONST_DOUBLEs holding integers.  */
+	  gcc_assert (GET_MODE (x) != VOIDmode);
+	  if (aarch64_float_const_zero_rtx_p (x))
 	    {
 	      fputc ('0', f);
 	      break;
diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index b041a1e..0fac0a7 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -863,6 +863,8 @@ extern enum aarch64_code_model aarch64_cmodel;
   (aarch64_cmodel == AARCH64_CMODEL_TINY		\
    || aarch64_cmodel == AARCH64_CMODEL_TINY_PIC)
 
+#define TARGET_SUPPORTS_WIDE_INT 1
+
 /* Modes valid for AdvSIMD D registers, i.e. that fit in half a Q register.  */
 #define AARCH64_VALID_SIMD_DREG_MODE(MODE) \
   ((MODE) == V2SImode || (MODE) == V4HImode || (MODE) == V8QImode \
diff --git a/gcc/config/aarch64/predicates.md b/gcc/config/aarch64/predicates.md
index 1bcbf62..8775460 100644
--- a/gcc/config/aarch64/predicates.md
+++ b/gcc/config/aarch64/predicates.md
@@ -32,7 +32,7 @@ (define_predicate "aarch64_call_insn_operand"
 
 ;; Return true if OP a (const_int 0) operand.
 (define_predicate "const0_operand"
-  (and (match_code "const_int, const_double")
+  (and (match_code "const_int")
        (match_test "op == CONST0_RTX (mode)")))
 
 (define_predicate "aarch64_ccmp_immediate"
diff --git a/gcc/testsuite/gcc.target/aarch64/pr68129_1.c b/gcc/testsuite/gcc.target/aarch64/pr68129_1.c
new file mode 100644
index 0000000..112331e
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/pr68129_1.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-O -fno-split-wide-types" } */
+
+typedef int V __attribute__ ((vector_size (8 * sizeof (int))));
+
+void
+foo (V *p, V *q)
+{
+  *p = (*p == *q);
+}

Re: [PATCH][AArch64] PR target/68129: Define TARGET_SUPPORTS_WIDE_INT

[Marcus Shawcroft]

On 9 November 2015 at 11:32, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote:

> 2015-11-09  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>
>     PR target/68129
>     * config/aarch64/aarch64.h (TARGET_SUPPORTS_WIDE_INT): Define to 1.
>     * config/aarch64/aarch64.c (aarch64_print_operand, CONST_DOUBLE):
>     Delete VOIDmode case.  Assert that mode is not VOIDmode.
>     * config/aarch64/predicates.md (const0_operand): Remove const_double
>     match.
>
> 2015-11-09  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>
>     PR target/68129
>     * gcc.target/aarch64/pr68129_1.c: New test.

Hi, This test isn't aarch64 specific, does it need to be in gcc.target/aarch64 ?

Cheers
/Marcus

Re: [PATCH][AArch64] PR target/68129: Define TARGET_SUPPORTS_WIDE_INT

[Kyrill Tkachov]

[-- Attachment #1: Type: text/plain, Size: 1374 bytes --]


On 09/11/15 15:34, Marcus Shawcroft wrote:
> On 9 November 2015 at 11:32, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote:
>
>> 2015-11-09  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>>
>>      PR target/68129
>>      * config/aarch64/aarch64.h (TARGET_SUPPORTS_WIDE_INT): Define to 1.
>>      * config/aarch64/aarch64.c (aarch64_print_operand, CONST_DOUBLE):
>>      Delete VOIDmode case.  Assert that mode is not VOIDmode.
>>      * config/aarch64/predicates.md (const0_operand): Remove const_double
>>      match.
>>
>> 2015-11-09  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>>
>>      PR target/68129
>>      * gcc.target/aarch64/pr68129_1.c: New test.
> Hi, This test isn't aarch64 specific, does it need to be in gcc.target/aarch64 ?

Not really, here is the patch with the test in gcc.dg/ if that's preferred.

Thanks,
Kyrill

2015-11-09  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     PR target/68129
     * config/aarch64/aarch64.h (TARGET_SUPPORTS_WIDE_INT): Define to 1.
     * config/aarch64/aarch64.c (aarch64_print_operand, CONST_DOUBLE):
     Delete VOIDmode case.  Assert that mode is not VOIDmode.
     * config/aarch64/predicates.md (const0_operand): Remove const_double
     match.

2015-11-09  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     PR target/68129
     * gcc.dg/pr68129_1.c: New test.

> Cheers
> /Marcus
>


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: aarch64-target-supports-wide-int.patch --]
[-- Type: text/x-patch; name=aarch64-target-supports-wide-int.patch, Size: 2495 bytes --]

commit 623ffaa527b17ad01179c30c1d4a9911243f818a
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date:   Wed Oct 28 10:49:44 2015 +0000

    [AArch64] PR target/68129: Define TARGET_SUPPORTS_WIDE_INT

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index ce155dc..927b72a 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -4403,11 +4403,10 @@ aarch64_print_operand (FILE *f, rtx x, char code)
 	  break;
 
 	case CONST_DOUBLE:
-	  /* CONST_DOUBLE can represent a double-width integer.
-	     In this case, the mode of x is VOIDmode.  */
-	  if (GET_MODE (x) == VOIDmode)
-	    ; /* Do Nothing.  */
-	  else if (aarch64_float_const_zero_rtx_p (x))
+	  /* Since we define TARGET_SUPPORTS_WIDE_INT we shouldn't ever
+	     be getting CONST_DOUBLEs holding integers.  */
+	  gcc_assert (GET_MODE (x) != VOIDmode);
+	  if (aarch64_float_const_zero_rtx_p (x))
 	    {
 	      fputc ('0', f);
 	      break;
diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index b041a1e..0fac0a7 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -863,6 +863,8 @@ extern enum aarch64_code_model aarch64_cmodel;
   (aarch64_cmodel == AARCH64_CMODEL_TINY		\
    || aarch64_cmodel == AARCH64_CMODEL_TINY_PIC)
 
+#define TARGET_SUPPORTS_WIDE_INT 1
+
 /* Modes valid for AdvSIMD D registers, i.e. that fit in half a Q register.  */
 #define AARCH64_VALID_SIMD_DREG_MODE(MODE) \
   ((MODE) == V2SImode || (MODE) == V4HImode || (MODE) == V8QImode \
diff --git a/gcc/config/aarch64/predicates.md b/gcc/config/aarch64/predicates.md
index 1bcbf62..8775460 100644
--- a/gcc/config/aarch64/predicates.md
+++ b/gcc/config/aarch64/predicates.md
@@ -32,7 +32,7 @@ (define_predicate "aarch64_call_insn_operand"
 
 ;; Return true if OP a (const_int 0) operand.
 (define_predicate "const0_operand"
-  (and (match_code "const_int, const_double")
+  (and (match_code "const_int")
        (match_test "op == CONST0_RTX (mode)")))
 
 (define_predicate "aarch64_ccmp_immediate"
diff --git a/gcc/testsuite/gcc.dg/pr68129_1.c b/gcc/testsuite/gcc.dg/pr68129_1.c
new file mode 100644
index 0000000..112331e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr68129_1.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-O -fno-split-wide-types" } */
+
+typedef int V __attribute__ ((vector_size (8 * sizeof (int))));
+
+void
+foo (V *p, V *q)
+{
+  *p = (*p == *q);
+}

Re: [PATCH][AArch64] PR target/68129: Define TARGET_SUPPORTS_WIDE_INT

[Marcus Shawcroft]

On 9 November 2015 at 15:45, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote:
>
> On 09/11/15 15:34, Marcus Shawcroft wrote:
>>
>> On 9 November 2015 at 11:32, Kyrill Tkachov <kyrylo.tkachov@arm.com>
>> wrote:
>>
>>> 2015-11-09  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>>>
>>>      PR target/68129
>>>      * config/aarch64/aarch64.h (TARGET_SUPPORTS_WIDE_INT): Define to 1.
>>>      * config/aarch64/aarch64.c (aarch64_print_operand, CONST_DOUBLE):
>>>      Delete VOIDmode case.  Assert that mode is not VOIDmode.
>>>      * config/aarch64/predicates.md (const0_operand): Remove const_double
>>>      match.
>>>
>>> 2015-11-09  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>>>
>>>      PR target/68129
>>>      * gcc.target/aarch64/pr68129_1.c: New test.
>>
>> Hi, This test isn't aarch64 specific, does it need to be in
>> gcc.target/aarch64 ?
>
>
> Not really, here is the patch with the test in gcc.dg/ if that's preferred.


OK /Marcus

Re: [PATCH][AArch64] PR target/68129: Define TARGET_SUPPORTS_WIDE_INT

[Mike Stump]

On Nov 9, 2015, at 3:32 AM, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote:
> The aarch64 port does not define TARGET_SUPPORTS_WIDE_INT.

> Ok for trunk and GCC 5?

:-)  I’d endorse it, but, best left to the target folks.