diff mbox

[v2,AArch64,1/6] Reimplement scalar fixed-point intrinsics

Message ID 083e6b20-d439-230a-00c9-d311b702be1b@foss.arm.com
State New
Headers show

Commit Message

Jiong Wang June 6, 2016, 1:38 p.m. UTC 
On 27/05/16 17:52, Jiong Wang wrote:
>
>
> On 27/05/16 14:03, James Greenhalgh wrote:
>> On Tue, May 24, 2016 at 09:23:36AM +0100, Jiong Wang wrote:
>>>          * config/aarch64/aarch64-simd-builtins.def: Rename to
>>>          aarch64-builtins.def.
>> Why? We already have some number of intrinsics in here that are not
>> strictly SIMD, but I don't see the value in the rename?
>
> Mostly because this builtin infrastructure is handy that I want to
> implement some vfp builtins in this .def file instead of implement those
> raw structure inside aarch64-builtins.c.
>
> And there maybe more and more such builtins in the future, so I renamed
> this file.
>
>
> Is this OK?
>
>>> +(define_int_iterator FCVT_FIXED2F_SCALAR [UNSPEC_SCVTF_SCALAR 
>>> UNSPEC_UCVTF_SCALAR])
>> Again, do we need the "SCALAR" versions at all?
>
> That's because for scalar fixed-point conversion, we have two types of
> instructions to support this.
>
>   * scalar instruction from vfp
>   * scalar variant instruction from simd
>
> One is guarded by TARGET_FLOAT, the other is guarded by TARGET_SIMD, and
> their instruction format is different, so I want to keep them in
> aarch64.md and aarch64-simd.md seperately.
>
> The other reason is these two use different patterns:
>
>   * vfp scalar support conversion between different size, for example,
>     SF->DI, DF->SI, so it's using two mode iterators, GPI and GPF, and
>     is utilizing the product of the two to cover all supported
>     conversions, sfsi, sfdi, dfsi, dfdi, sisf, sidf, disf, didf.
>
>   * simd scalar only support conversion between same size that single
>     mode iterator is used to cover sfsi, sisf, dfdi, didf.
>
> For intrinsics implementation, I used builtins backed by vfp scalar
> instead of simd scalar which requires the input sitting inside vector 
> register.
>
> I remember the simd scalar pattern was here because it's anyway needed
> by patch [2/6] which extends it's modes naturally to vector modes. I was
> thinking it's better to keep simd scalar variant with this scalar
> intrinsics enable patch.
>
> Is this OK?
>
> Thanks.

I updated this patch set with the following modifications:

   * drop the renaming of aarch64-builtins.def
   * implemented vrsqrts_f64, vrsqrte_f64, vabd_f64, vpadds_f32 as I am here.


OK for trunk?

gcc/
2016-06-06  Jiong Wang<jiong.wang@arm.com>

         * config/aarch64/aarch64-builtins.c (TYPES_BINOP_USS): New
         (TYPES_BINOP_SUS): Likewise.
         (aarch64_simd_builtin_data): Update include file name.
         (aarch64_builtins): Likewise.
         * config/aarch64/aarch64-simd-builtins.def (scvtf): New entries
         for conversion between scalar float-point and fixed-point.
         (ucvtf): Likewise.
         (fcvtzs): Likewise.
         (fcvtzu): Likewise.
         * config/aarch64/aarch64.md
         (<FCVT_F2FIXED:fcvt_fixed_insn><GPF:mode>3: New
         pattern for conversion between scalar float to fixed-pointer.
         (<FCVT_FIXED2F:fcvt_fixed_insn><GPI:mode>: Likewise.
         (UNSPEC_FCVTZS): New UNSPEC enumeration.
         (UNSPEC_FCVTZU): Likewise.
         (UNSPEC_SCVTF): Likewise.
         (UNSPEC_UCVTF): Likewise.
         * config/aarch64/arm_neon.h (vcvtd_n_f64_s64): Remove inline assembly.  Use
         builtin.
         (vcvtd_n_f64_u64): Likewise.
         (vcvtd_n_s64_f64): Likewise.
         (vcvtd_n_u64_f64): Likewise.
         (vcvtd_n_f32_s32): Likewise.
         (vcvts_n_f32_u32): Likewise.
         (vcvtd_n_s32_f32): Likewise.
         (vcvts_n_u32_f32): Likewise.
         * config/aarch64/iterators.md (fcvt_target): Support integer to float mapping.
         (FCVT_TARGET): Likewise.
         (FCVT_FIXED2F): New iterator.
         (FCVT_F2FIXED): Likewise.
         (fcvt_fixed_insn): New define_int_attr.

Comments

James Greenhalgh June 8, 2016, 9:47 a.m. UTC | #1 
On Mon, Jun 06, 2016 at 02:38:58PM +0100, Jiong Wang wrote:
> On 27/05/16 17:52, Jiong Wang wrote:
> >
> >
> >On 27/05/16 14:03, James Greenhalgh wrote:
> >>On Tue, May 24, 2016 at 09:23:36AM +0100, Jiong Wang wrote:
> >>>         * config/aarch64/aarch64-simd-builtins.def: Rename to
> >>>         aarch64-builtins.def.
> >>Why? We already have some number of intrinsics in here that are not
> >>strictly SIMD, but I don't see the value in the rename?
> >
> >Mostly because this builtin infrastructure is handy that I want to
> >implement some vfp builtins in this .def file instead of implement those
> >raw structure inside aarch64-builtins.c.
> >
> >And there maybe more and more such builtins in the future, so I renamed
> >this file.
> >
> >
> >Is this OK?
> >
> >>>+(define_int_iterator FCVT_FIXED2F_SCALAR [UNSPEC_SCVTF_SCALAR
> >>>UNSPEC_UCVTF_SCALAR])
> >>Again, do we need the "SCALAR" versions at all?
> >
> >That's because for scalar fixed-point conversion, we have two types of
> >instructions to support this.
> >
> >  * scalar instruction from vfp
> >  * scalar variant instruction from simd
> >
> >One is guarded by TARGET_FLOAT, the other is guarded by TARGET_SIMD, and
> >their instruction format is different, so I want to keep them in
> >aarch64.md and aarch64-simd.md seperately.
> >
> >The other reason is these two use different patterns:
> >
> >  * vfp scalar support conversion between different size, for example,
> >    SF->DI, DF->SI, so it's using two mode iterators, GPI and GPF, and
> >    is utilizing the product of the two to cover all supported
> >    conversions, sfsi, sfdi, dfsi, dfdi, sisf, sidf, disf, didf.
> >
> >  * simd scalar only support conversion between same size that single
> >    mode iterator is used to cover sfsi, sisf, dfdi, didf.
> >
> >For intrinsics implementation, I used builtins backed by vfp scalar
> >instead of simd scalar which requires the input sitting inside
> >vector register.
> >
> >I remember the simd scalar pattern was here because it's anyway needed
> >by patch [2/6] which extends it's modes naturally to vector modes. I was
> >thinking it's better to keep simd scalar variant with this scalar
> >intrinsics enable patch.
> >
> >Is this OK?

This is OK. Just watch the length of some of your ChangeLog lines when you
commit.

Thanks,
James

> gcc/
> 2016-06-06  Jiong Wang<jiong.wang@arm.com>
> 
>         * config/aarch64/aarch64-builtins.c (TYPES_BINOP_USS): New
>         (TYPES_BINOP_SUS): Likewise.
>         (aarch64_simd_builtin_data): Update include file name.
>         (aarch64_builtins): Likewise.
>         * config/aarch64/aarch64-simd-builtins.def (scvtf): New entries
>         for conversion between scalar float-point and fixed-point.
>         (ucvtf): Likewise.
>         (fcvtzs): Likewise.
>         (fcvtzu): Likewise.
>         * config/aarch64/aarch64.md
>         (<FCVT_F2FIXED:fcvt_fixed_insn><GPF:mode>3: New
>         pattern for conversion between scalar float to fixed-pointer.
>         (<FCVT_FIXED2F:fcvt_fixed_insn><GPI:mode>: Likewise.
>         (UNSPEC_FCVTZS): New UNSPEC enumeration.
>         (UNSPEC_FCVTZU): Likewise.
>         (UNSPEC_SCVTF): Likewise.
>         (UNSPEC_UCVTF): Likewise.
>         * config/aarch64/arm_neon.h (vcvtd_n_f64_s64): Remove inline assembly.  Use
>         builtin.
>         (vcvtd_n_f64_u64): Likewise.
>         (vcvtd_n_s64_f64): Likewise.
>         (vcvtd_n_u64_f64): Likewise.
>         (vcvtd_n_f32_s32): Likewise.
>         (vcvts_n_f32_u32): Likewise.
>         (vcvtd_n_s32_f32): Likewise.
>         (vcvts_n_u32_f32): Likewise.
>         * config/aarch64/iterators.md (fcvt_target): Support integer to float mapping.
>         (FCVT_TARGET): Likewise.
>         (FCVT_FIXED2F): New iterator.
>         (FCVT_F2FIXED): Likewise.
>         (fcvt_fixed_insn): New define_int_attr.
>
diff mbox

Patch

diff --git a/gcc/config/aarch64/aarch64-builtins.c b/gcc/config/aarch64/aarch64-builtins.c
index 5573903fe0a1f3d1ffc58c36992bd46cd0cb4dad..262ea1c519f4f01a1a0726296994e40a48f26680 100644
--- a/gcc/config/aarch64/aarch64-builtins.c
+++ b/gcc/config/aarch64/aarch64-builtins.c
@@ -139,6 +139,14 @@  aarch64_types_binop_ssu_qualifiers[SIMD_MAX_BUILTIN_ARGS]
   = { qualifier_none, qualifier_none, qualifier_unsigned };
 #define TYPES_BINOP_SSU (aarch64_types_binop_ssu_qualifiers)
 static enum aarch64_type_qualifiers
+aarch64_types_binop_uss_qualifiers[SIMD_MAX_BUILTIN_ARGS]
+  = { qualifier_unsigned, qualifier_none, qualifier_none };
+#define TYPES_BINOP_USS (aarch64_types_binop_uss_qualifiers)
+static enum aarch64_type_qualifiers
+aarch64_types_binop_sus_qualifiers[SIMD_MAX_BUILTIN_ARGS]
+  = { qualifier_none, qualifier_unsigned, qualifier_none };
+#define TYPES_BINOP_SUS (aarch64_types_binop_sus_qualifiers)
+static enum aarch64_type_qualifiers
 aarch64_types_binopp_qualifiers[SIMD_MAX_BUILTIN_ARGS]
   = { qualifier_poly, qualifier_poly, qualifier_poly };
 #define TYPES_BINOPP (aarch64_types_binopp_qualifiers)
diff --git a/gcc/config/aarch64/aarch64-simd-builtins.def b/gcc/config/aarch64/aarch64-simd-builtins.def
index dd045792b21f84b9587be08a07db0e0081e0c484..0b2f0631c740558c62cffe5715eaffa5ad0557a9 100644
--- a/gcc/config/aarch64/aarch64-simd-builtins.def
+++ b/gcc/config/aarch64/aarch64-simd-builtins.def
@@ -445,3 +445,9 @@ 
   /* Implemented by aarch64_sqrdml<SQRDMLH_AS:rdma_as>h_laneq<mode>.  */
   BUILTIN_VSDQ_HSI (QUADOP_LANE, sqrdmlah_laneq, 0)
   BUILTIN_VSDQ_HSI (QUADOP_LANE, sqrdmlsh_laneq, 0)
+
+  /* Implemented by <FCVT_F2FIXED/FIXED2F:fcvt_fixed_insn><*><*>3.  */
+  BUILTIN_GPI (BINOP, scvtf, 3)
+  BUILTIN_GPI (BINOP_SUS, ucvtf, 3)
+  BUILTIN_GPF (BINOP, fcvtzs, 3)
+  BUILTIN_GPF (BINOP_USS, fcvtzu, 3)
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index f04f7daed276ad53619623405c384ffe300fc8c1..8e6a082e91fcad18cc891c83209b061eef6449e0 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -75,6 +75,8 @@ 
     UNSPEC_CRC32H
     UNSPEC_CRC32W
     UNSPEC_CRC32X
+    UNSPEC_FCVTZS
+    UNSPEC_FCVTZU
     UNSPEC_URECPE
     UNSPEC_FRECPE
     UNSPEC_FRECPS
@@ -105,6 +107,7 @@ 
     UNSPEC_NOP
     UNSPEC_PRLG_STK
     UNSPEC_RBIT
+    UNSPEC_SCVTF
     UNSPEC_SISD_NEG
     UNSPEC_SISD_SSHL
     UNSPEC_SISD_USHL
@@ -122,6 +125,7 @@ 
     UNSPEC_TLSLE24
     UNSPEC_TLSLE32
     UNSPEC_TLSLE48
+    UNSPEC_UCVTF
     UNSPEC_USHL_2S
     UNSPEC_VSTRUCTDUMMY
     UNSPEC_SP_SET
@@ -4620,6 +4624,36 @@ 
   [(set_attr "type" "f_cvti2f")]
 )
 
+;; Convert between fixed-point and floating-point (scalar modes)
+
+(define_insn "<FCVT_F2FIXED:fcvt_fixed_insn><GPF:mode>3"
+  [(set (match_operand:<GPF:FCVT_TARGET> 0 "register_operand" "=r, w")
+	(unspec:<GPF:FCVT_TARGET> [(match_operand:GPF 1 "register_operand" "w, w")
+				   (match_operand:SI 2 "immediate_operand" "i, i")]
+	 FCVT_F2FIXED))]
+  ""
+  "@
+   <FCVT_F2FIXED:fcvt_fixed_insn>\t%<w1>0, %<s>1, #%2
+   <FCVT_F2FIXED:fcvt_fixed_insn>\t%<s>0, %<s>1, #%2"
+  [(set_attr "type" "f_cvtf2i, neon_fp_to_int_<GPF:Vetype>")
+   (set_attr "fp" "yes, *")
+   (set_attr "simd" "*, yes")]
+)
+
+(define_insn "<FCVT_FIXED2F:fcvt_fixed_insn><GPI:mode>3"
+  [(set (match_operand:<GPI:FCVT_TARGET> 0 "register_operand" "=w, w")
+	(unspec:<GPI:FCVT_TARGET> [(match_operand:GPI 1 "register_operand" "r, w")
+				   (match_operand:SI 2 "immediate_operand" "i, i")]
+	 FCVT_FIXED2F))]
+  ""
+  "@
+   <FCVT_FIXED2F:fcvt_fixed_insn>\t%<s>0, %<w1>1, #%2
+   <FCVT_FIXED2F:fcvt_fixed_insn>\t%<s>0, %<s>1, #%2"
+  [(set_attr "type" "f_cvti2f, neon_int_to_fp_<GPI:Vetype>")
+   (set_attr "fp" "yes, *")
+   (set_attr "simd" "*, yes")]
+)
+
 ;; -------------------------------------------------------------------
 ;; Floating-point arithmetic
 ;; -------------------------------------------------------------------
diff --git a/gcc/config/aarch64/arm_neon.h b/gcc/config/aarch64/arm_neon.h
index d20caf0919356eb7a87e7c7a9cd336d8408db35b..8a0fba6513e572ede9f2e4aaf8d29baf6baf683d 100644
--- a/gcc/config/aarch64/arm_neon.h
+++ b/gcc/config/aarch64/arm_neon.h
@@ -6073,54 +6073,6 @@  vaddlvq_u32 (uint32x4_t a)
        result;                                                          \
      })
 
-#define vcvtd_n_f64_s64(a, b)                                           \
-  __extension__                                                         \
-    ({                                                                  \
-       int64_t a_ = (a);                                                \
-       float64_t result;                                                \
-       __asm__ ("scvtf %d0,%d1,%2"                                      \
-                : "=w"(result)                                          \
-                : "w"(a_), "i"(b)                                       \
-                : /* No clobbers */);                                   \
-       result;                                                          \
-     })
-
-#define vcvtd_n_f64_u64(a, b)                                           \
-  __extension__                                                         \
-    ({                                                                  \
-       uint64_t a_ = (a);                                               \
-       float64_t result;                                                \
-       __asm__ ("ucvtf %d0,%d1,%2"                                      \
-                : "=w"(result)                                          \
-                : "w"(a_), "i"(b)                                       \
-                : /* No clobbers */);                                   \
-       result;                                                          \
-     })
-
-#define vcvtd_n_s64_f64(a, b)                                           \
-  __extension__                                                         \
-    ({                                                                  \
-       float64_t a_ = (a);                                              \
-       int64_t result;                                                  \
-       __asm__ ("fcvtzs %d0,%d1,%2"                                     \
-                : "=w"(result)                                          \
-                : "w"(a_), "i"(b)                                       \
-                : /* No clobbers */);                                   \
-       result;                                                          \
-     })
-
-#define vcvtd_n_u64_f64(a, b)                                           \
-  __extension__                                                         \
-    ({                                                                  \
-       float64_t a_ = (a);                                              \
-       uint64_t result;                                                 \
-       __asm__ ("fcvtzu %d0,%d1,%2"                                     \
-                : "=w"(result)                                          \
-                : "w"(a_), "i"(b)                                       \
-                : /* No clobbers */);                                   \
-       result;                                                          \
-     })
-
 #define vcvtq_n_f32_s32(a, b)                                           \
   __extension__                                                         \
     ({                                                                  \
@@ -6217,54 +6169,6 @@  vaddlvq_u32 (uint32x4_t a)
        result;                                                          \
      })
 
-#define vcvts_n_f32_s32(a, b)                                           \
-  __extension__                                                         \
-    ({                                                                  \
-       int32_t a_ = (a);                                                \
-       float32_t result;                                                \
-       __asm__ ("scvtf %s0,%s1,%2"                                      \
-                : "=w"(result)                                          \
-                : "w"(a_), "i"(b)                                       \
-                : /* No clobbers */);                                   \
-       result;                                                          \
-     })
-
-#define vcvts_n_f32_u32(a, b)                                           \
-  __extension__                                                         \
-    ({                                                                  \
-       uint32_t a_ = (a);                                               \
-       float32_t result;                                                \
-       __asm__ ("ucvtf %s0,%s1,%2"                                      \
-                : "=w"(result)                                          \
-                : "w"(a_), "i"(b)                                       \
-                : /* No clobbers */);                                   \
-       result;                                                          \
-     })
-
-#define vcvts_n_s32_f32(a, b)                                           \
-  __extension__                                                         \
-    ({                                                                  \
-       float32_t a_ = (a);                                              \
-       int32_t result;                                                  \
-       __asm__ ("fcvtzs %s0,%s1,%2"                                     \
-                : "=w"(result)                                          \
-                : "w"(a_), "i"(b)                                       \
-                : /* No clobbers */);                                   \
-       result;                                                          \
-     })
-
-#define vcvts_n_u32_f32(a, b)                                           \
-  __extension__                                                         \
-    ({                                                                  \
-       float32_t a_ = (a);                                              \
-       uint32_t result;                                                 \
-       __asm__ ("fcvtzu %s0,%s1,%2"                                     \
-                : "=w"(result)                                          \
-                : "w"(a_), "i"(b)                                       \
-                : /* No clobbers */);                                   \
-       result;                                                          \
-     })
-
 __extension__ static __inline float32x2_t __attribute__ ((__always_inline__))
 vcvtx_f32_f64 (float64x2_t a)
 {
@@ -12830,6 +12734,58 @@  vcvt_high_f64_f32 (float32x4_t __a)
   return __builtin_aarch64_vec_unpacks_hi_v4sf (__a);
 }
 
+/* vcvt (<u>fixed-point -> float).  */
+
+__extension__ static __inline float64_t __attribute__ ((__always_inline__))
+vcvtd_n_f64_s64 (int64_t __a, const int __b)
+{
+  return __builtin_aarch64_scvtfdi (__a, __b);
+}
+
+__extension__ static __inline float64_t __attribute__ ((__always_inline__))
+vcvtd_n_f64_u64 (uint64_t __a, const int __b)
+{
+  return __builtin_aarch64_ucvtfdi_sus (__a, __b);
+}
+
+__extension__ static __inline float32_t __attribute__ ((__always_inline__))
+vcvts_n_f32_s32 (int32_t __a, const int __b)
+{
+  return __builtin_aarch64_scvtfsi (__a, __b);
+}
+
+__extension__ static __inline float32_t __attribute__ ((__always_inline__))
+vcvts_n_f32_u32 (uint32_t __a, const int __b)
+{
+  return __builtin_aarch64_ucvtfsi_sus (__a, __b);
+}
+
+/* vcvt (float -> <u>fixed-point).  */
+
+__extension__ static __inline int64_t __attribute__ ((__always_inline__))
+vcvtd_n_s64_f64 (float64_t __a, const int __b)
+{
+  return __builtin_aarch64_fcvtzsdf (__a, __b);
+}
+
+__extension__ static __inline uint64_t __attribute__ ((__always_inline__))
+vcvtd_n_u64_f64 (float64_t __a, const int __b)
+{
+  return __builtin_aarch64_fcvtzudf_uss (__a, __b);
+}
+
+__extension__ static __inline int32_t __attribute__ ((__always_inline__))
+vcvts_n_s32_f32 (float32_t __a, const int __b)
+{
+  return __builtin_aarch64_fcvtzssf (__a, __b);
+}
+
+__extension__ static __inline uint32_t __attribute__ ((__always_inline__))
+vcvts_n_u32_f32 (float32_t __a, const int __b)
+{
+  return __builtin_aarch64_fcvtzusf_uss (__a, __b);
+}
+
 /* vcvt  (<u>int -> float)  */
 
 __extension__ static __inline float64_t __attribute__ ((__always_inline__))
diff --git a/gcc/config/aarch64/iterators.md b/gcc/config/aarch64/iterators.md
index 43b22d81cda30398564af2f2fcaefceb215ec04c..2d59bed99b9d269c656e5c451246a16a7e13b8b8 100644
--- a/gcc/config/aarch64/iterators.md
+++ b/gcc/config/aarch64/iterators.md
@@ -648,8 +648,11 @@ 
 (define_mode_attr atomic_sfx
   [(QI "b") (HI "h") (SI "") (DI "")])
 
-(define_mode_attr fcvt_target [(V2DF "v2di") (V4SF "v4si") (V2SF "v2si") (SF "si") (DF "di")])
-(define_mode_attr FCVT_TARGET [(V2DF "V2DI") (V4SF "V4SI") (V2SF "V2SI") (SF "SI") (DF "DI")])
+(define_mode_attr fcvt_target [(V2DF "v2di") (V4SF "v4si") (V2SF "v2si")
+			       (SF "si") (DF "di") (SI "sf") (DI "df")])
+(define_mode_attr FCVT_TARGET [(V2DF "V2DI") (V4SF "V4SI") (V2SF "V2SI")
+			       (SF "SI") (DF "DI") (SI "SF") (DI "DF")])
+
 
 ;; for the inequal width integer to fp conversions
 (define_mode_attr fcvt_iesize [(SF "di") (DF "si")])
@@ -1002,6 +1005,9 @@ 
 (define_int_iterator FCVT [UNSPEC_FRINTZ UNSPEC_FRINTP UNSPEC_FRINTM
 			    UNSPEC_FRINTA UNSPEC_FRINTN])
 
+(define_int_iterator FCVT_F2FIXED [UNSPEC_FCVTZS UNSPEC_FCVTZU])
+(define_int_iterator FCVT_FIXED2F [UNSPEC_SCVTF UNSPEC_UCVTF])
+
 (define_int_iterator FRECP [UNSPEC_FRECPE UNSPEC_FRECPX])
 
 (define_int_iterator CRC [UNSPEC_CRC32B UNSPEC_CRC32H UNSPEC_CRC32W
@@ -1138,6 +1144,11 @@ 
 			       (UNSPEC_FRINTP "ceil") (UNSPEC_FRINTM "floor")
 			       (UNSPEC_FRINTN "frintn")])
 
+(define_int_attr fcvt_fixed_insn [(UNSPEC_SCVTF "scvtf")
+				  (UNSPEC_UCVTF "ucvtf")
+				  (UNSPEC_FCVTZS "fcvtzs")
+				  (UNSPEC_FCVTZU "fcvtzu")])
+
 (define_int_attr perm_insn [(UNSPEC_ZIP1 "zip") (UNSPEC_ZIP2 "zip")
 			    (UNSPEC_TRN1 "trn") (UNSPEC_TRN2 "trn")
 			    (UNSPEC_UZP1 "uzp") (UNSPEC_UZP2 "uzp")])