diff mbox

[AArch64] Make integer vabs intrinsics UNSPECs

Message ID 1422437045-24696-1-git-send-email-james.greenhalgh@arm.com
State New
Headers show

Commit Message

James Greenhalgh Jan. 28, 2015, 9:24 a.m. UTC 
Hi,

I had proposed this patch back in May:

  https://gcc.gnu.org/ml/gcc-patches/2014-05/msg00077.html

And then Alan proposed a derivative in December, where we thought that
we would be fine without making vabs the intrinsics map to an unspec:

  https://gcc.gnu.org/ml/gcc-patches/2014-12/msg00253.html

The testcase added in this patch shows that this is incorrect,
we need to prevent the ABS that vabs currently generates from getting
combined in to any instructions like SABD. We don't need to disable
the SABD entirely as the behaviour on overflow isn't relevant outside
of the Neon/Advanced SIMD intrinsics.

In Alan's thread he pointed out that we don't have any fold rules for
abs anyway, so we don't lose much with this approach.

This has passed a bootstrap and test run on aarch64-none-linux-gnu.
It isn't strictly a regression, but as a back-end only bug, impacting
a corner case of Neon intrinsics, I think it is low risk.

With that in mind, OK for trunk?

Thanks,
James

---
gcc/

2015-01-28  James Greenhalgh  <james.greenhalgh@arm.com>

	* config/aarch64/aarch64-simd.md (aarch64_abs<mode>): New.
	* config/aarch64/aarch64-simd-builtins.def (abs): Split by
	integer and floating point variants.
	* config/aarch64/iterators.md (unspec): Add UNSPEC_ABS.

gcc/testsuite/

2015-01-28  James Greenhalgh  <james.greenhalgh@arm.com>

	* gcc.target/aarch64/abs_2.c: New.

Comments

Marcus Shawcroft Jan. 28, 2015, 9:32 a.m. UTC | #1 
On 28 January 2015 at 09:24, James Greenhalgh <james.greenhalgh@arm.com> wrote:

> 2015-01-28  James Greenhalgh  <james.greenhalgh@arm.com>
>
>         * config/aarch64/aarch64-simd.md (aarch64_abs<mode>): New.
>         * config/aarch64/aarch64-simd-builtins.def (abs): Split by
>         integer and floating point variants.
>         * config/aarch64/iterators.md (unspec): Add UNSPEC_ABS.
>
> gcc/testsuite/
>
> 2015-01-28  James Greenhalgh  <james.greenhalgh@arm.com>
>
>         * gcc.target/aarch64/abs_2.c: New.

OK /Marcus
diff mbox

Patch

diff --git a/gcc/config/aarch64/aarch64-simd-builtins.def b/gcc/config/aarch64/aarch64-simd-builtins.def
index 1a1520c..2c52b27 100644
--- a/gcc/config/aarch64/aarch64-simd-builtins.def
+++ b/gcc/config/aarch64/aarch64-simd-builtins.def
@@ -358,7 +358,8 @@ 
 
   /* Implemented by a mixture of abs2 patterns.  Note the DImode builtin is
      only ever used for the int64x1_t intrinsic, there is no scalar version.  */
-  BUILTIN_VALLDI (UNOP, abs, 2)
+  BUILTIN_VSDQ_I_DI (UNOP, abs, 0)
+  BUILTIN_VDQF (UNOP, abs, 2)
 
   VAR1 (UNOP, vec_unpacks_hi_, 10, v4sf)
   VAR1 (BINOP, float_truncate_hi_, 0, v4sf)
diff --git a/gcc/config/aarch64/aarch64-simd.md b/gcc/config/aarch64/aarch64-simd.md
index 17ac56c010f74f6d87b57dbbb93e6f7175ec7ce6..055757036d54d0d5cf5df4bd05419e39ea119f46 100644
--- a/gcc/config/aarch64/aarch64-simd.md
+++ b/gcc/config/aarch64/aarch64-simd.md
@@ -384,6 +384,19 @@  (define_insn "abs<mode>2"
   [(set_attr "type" "neon_abs<q>")]
 )
 
+;; The intrinsic version of integer ABS must not be allowed to
+;; combine with any operation with an integerated ABS step, such
+;; as SABD.
+(define_insn "aarch64_abs<mode>"
+  [(set (match_operand:VSDQ_I_DI 0 "register_operand" "=w")
+	  (unspec:VSDQ_I_DI
+	    [(match_operand:VSDQ_I_DI 1 "register_operand" "w")]
+	   UNSPEC_ABS))]
+  "TARGET_SIMD"
+  "abs\t%<v>0<Vmtype>, %<v>1<Vmtype>"
+  [(set_attr "type" "neon_abs<q>")]
+)
+
 (define_insn "abd<mode>_3"
   [(set (match_operand:VDQ_BHSI 0 "register_operand" "=w")
 	(abs:VDQ_BHSI (minus:VDQ_BHSI
diff --git a/gcc/config/aarch64/iterators.md b/gcc/config/aarch64/iterators.md
index 606ccc33eec3941715f0ed6f365d2e60551c7251..65aa1e8916ab2440afe336139d40b572b0ee878a 100644
--- a/gcc/config/aarch64/iterators.md
+++ b/gcc/config/aarch64/iterators.md
@@ -184,6 +184,7 @@  (define_c_enum "unspec"
  [
     UNSPEC_ASHIFT_SIGNED	; Used in aarch-simd.md.
     UNSPEC_ASHIFT_UNSIGNED	; Used in aarch64-simd.md.
+    UNSPEC_ABS		; Used in aarch64-simd.md.
     UNSPEC_FMAX		; Used in aarch64-simd.md.
     UNSPEC_FMAXNMV	; Used in aarch64-simd.md.
     UNSPEC_FMAXV	; Used in aarch64-simd.md.
diff --git a/gcc/testsuite/gcc.target/aarch64/abs_2.c b/gcc/testsuite/gcc.target/aarch64/abs_2.c
new file mode 100644
index 0000000..a10ccdd
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/abs_2.c
@@ -0,0 +1,31 @@ 
+/* { dg-do run } */
+/* { dg-options "-O2 --save-temps" } */
+
+#include "arm_neon.h"
+
+extern void abort (void);
+
+int
+main (int argc, char **argv)
+{
+  uint64_t got;
+  uint64_t exp = UINT64_C (0x0001000100003b9b);
+  int16x4_t val1 = vcreate_s16 (0x7fff800080007ffful);
+  int16x4_t val2 = vcreate_s16 (0x80007fff80004464ul);
+  int16x4_t result;
+  /* Avoid folding away the sub early.  */
+  asm volatile ("mov %d0, %0.d[0]":"+w"(val1));
+
+  /* Expect "result" = 0001000100003b9b.  */
+  result = vabs_s16 (vsub_s16 (val1, val2));
+
+  got = vget_lane_u64 (vreinterpret_u64_s16 (result), 0);
+  if (exp != got)
+    abort ();
+
+  return 0;
+}
+
+
+/* { dg-final { scan-assembler-not "sabd" } } */
+/* { dg-final { cleanup-saved-temps } } */