This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH 2/4 v2][AArch64] Add support for FCCMP
- From: Evandro Menezes <e dot menezes at samsung dot com>
- To: James Greenhalgh <james dot greenhalgh at arm dot com>
- Cc: Wilco Dijkstra <Wilco dot Dijkstra at arm dot com>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, nd <nd at arm dot com>, Andrew Pinski <pinskia at gmail dot com>
- Date: Thu, 21 Jan 2016 16:55:40 -0600
- Subject: Re: [PATCH 2/4 v2][AArch64] Add support for FCCMP
- Authentication-results: sourceware.org; auth=none
- References: <AM3PR08MB00888A3A9B83DA6250A5ED9183EE0 at AM3PR08MB0088 dot eurprd08 dot prod dot outlook dot com> <568C3D19 dot 908 at samsung dot com> <AM3PR08MB0088A372287FCB8D106A4DE383F40 at AM3PR08MB0088 dot eurprd08 dot prod dot outlook dot com> <568D7CBF dot 2070407 at samsung dot com> <20160121092402 dot GA8842 at arm dot com> <56A13867 dot 3010103 at samsung dot com> <20160121220712 dot GA25892 at arm dot com>
On 01/21/16 16:07, James Greenhalgh wrote:
On Thu, Jan 21, 2016 at 01:58:31PM -0600, Evandro Menezes wrote:
Hi, James.
On 01/21/16 03:24, James Greenhalgh wrote:
On Wed, Jan 06, 2016 at 02:44:47PM -0600, Evandro Menezes wrote:
On 01/06/2016 06:04 AM, Wilco Dijkstra wrote:
Here's what I had in mind when I inquired about distinguishing FCMP from
FCCMP. As you can see in the patch, Exynos is the only target that
cares about it, but I wonder if ThunderX or Xgene would too.
What do you think?
The new attributes look fine (I've got a similar outstanding change), however
please don't add them to non-AArch64 cores. We only need it for thunderx.md,
cortex-a53.md, cortex-a57.md, xgene1.md and exynos-m1.md.
Add support for the FCCMP insn types
2016-01-04 Evandro Menezes <e.menezes@samsung.com>
gcc/
* config/aarch64/aarch64.md (fccmp): Change insn type.
(fccmpe): Likewise.
* config/aarch64/thunderx.md (thunderx_fcmp): Add
"fccmp{s,d}" types.
* config/arm/cortex-a53.md (cortex_a53_fpalu): Likewise.
* config/arm/cortex-a57.md (cortex_a57_fp_cmp): Likewise.
* config/arm/xgene1.md (xgene1_fcmp): Likewise.
* config/arm/exynos-m1.md (exynos_m1_fp_ccmp): New insn
reservation.
* config/arm/types.md (fccmps): Add new insn type.
(fccmpd): Likewise.
Got it. Here's an updated patch. Again, assuming that your
original patch is in place. Perhaps you can build on it.
If we don't have any targets which care about the fccmps/fccmpd split in
the code base, do we really need it? Can we just follow the example of
fcsel?
The Exynos M1 does care about the difference between FCMP and FCCMP,
as can be seen in the patch.
More explicitly:
(define_insn_reservation "exynos_m1_fp_cmp" 4
(and (eq_attr "tune" "exynosm1")
(eq_attr "type" "fcmps, fcmpd"))
"em1_nmisc")
(define_insn_reservation "exynos_m1_fp_ccmp" 7
(and (eq_attr "tune" "exynosm1")
(eq_attr "type" "fccmps, fccmpd"))
"em1_st, em1_nmisc")
I think I was unclear. Your exynos-m1 model cares about splitting fcmp[s/d]
and fccmp, but it doesn't care about splitting fccmp in to fccmps/fccmpd. It
is the split to fccmps/fccmpd that I think is unneccesary at this time.
Indeed. However, it seems to me that the jury is still out about the
{s,d} suffix, isn't it? Otherwise, whatever others deem better. I
myself am agnostic about it.
diff --git a/gcc/config/arm/types.md b/gcc/config/arm/types.md
index 321ff89..daf7162 100644
--- a/gcc/config/arm/types.md
+++ b/gcc/config/arm/types.md
@@ -70,6 +70,7 @@
; f_rint[d,s] double/single floating point rount to integral.
; f_store[d,s] double/single store to memory. Used for VFP unit.
; fadd[d,s] double/single floating-point scalar addition.
+; fccmp[d,s] double/single floating-point conditional compare.
Can we follow the convention fcsel uses of calling out "From ARMv8-A:"
for this type?
I'm not sure I follow. Though I didn't refer to the ISA spec, I
used the description from it for the *fccmp* type.
Something like:
; fccmp From ARMv8-A: floating point conditional compare.
Just to capture that this instruction is only available for cores implementing
ARMv8-A.
Got it.
Let me try this again:
Add support for the FCCMP insn types
2016-01-21 Evandro Menezes <e.menezes@samsung.com>
gcc/
* config/aarch64/aarch64.md (fccmp): Change insn type.
(fccmpe): Likewise.
* config/aarch64/thunderx.md (thunderx_fcmp): Add
"fccmp{s,d}" types.
* config/arm/cortex-a53.md (cortex_a53_fpalu): Likewise.
* config/arm/cortex-a57.md (cortex_a57_fp_cmp): Likewise.
* config/arm/xgene1.md (xgene1_fcmp): Likewise.
* config/arm/exynos-m1.md (exynos_m1_fp_ccmp): New insn
reservation.
* config/arm/types.md (fccmps): Add new insn type.
(fccmpd): Likewise.
Thank you,
--
Evandro Menezes
>From 14874dec3257c7b59aed4b7c610305f76bbbcf33 Mon Sep 17 00:00:00 2001
From: Evandro Menezes <e.menezes@samsung.com>
Date: Mon, 4 Jan 2016 18:44:30 -0600
Subject: [PATCH] Add support for the FCCMP insn types
2016-01-21 Evandro Menezes <e.menezes@samsung.com>
gcc/
* config/aarch64/aarch64.md (fccmp): Change insn type.
(fccmpe): Likewise.
* config/aarch64/thunderx.md (thunderx_fcmp): Add "fccmp{s,d}" types.
* config/arm/cortex-a53.md (cortex_a53_fpalu): Likewise.
* config/arm/cortex-a57.md (cortex_a57_fp_cmp): Likewise.
* config/arm/xgene1.md (xgene1_fcmp): Likewise.
* config/arm/exynos-m1.md (exynos_m1_fp_ccmp): New insn reservation.
* config/arm/types.md (fccmps): Add new insn type.
(fccmpd): Likewise.
---
gcc/config/aarch64/aarch64.md | 4 ++--
gcc/config/aarch64/thunderx.md | 2 +-
gcc/config/arm/cortex-a53.md | 4 ++--
gcc/config/arm/cortex-a57.md | 2 +-
gcc/config/arm/exynos-m1.md | 5 +++++
gcc/config/arm/types.md | 3 +++
gcc/config/arm/xgene1.md | 2 +-
7 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 2f543aa..032b342 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -301,7 +301,7 @@
(match_operand 5 "immediate_operand")))]
"TARGET_FLOAT"
"fccmp\\t%<s>2, %<s>3, %k5, %m4"
- [(set_attr "type" "fcmp<s>")]
+ [(set_attr "type" "fccmp<s>")]
)
(define_insn "fccmpe<mode>"
@@ -316,7 +316,7 @@
(match_operand 5 "immediate_operand")))]
"TARGET_FLOAT"
"fccmpe\\t%<s>2, %<s>3, %k5, %m4"
- [(set_attr "type" "fcmp<s>")]
+ [(set_attr "type" "fccmp<s>")]
)
;; Expansion of signed mod by a power of 2 using CSNEG.
diff --git a/gcc/config/aarch64/thunderx.md b/gcc/config/aarch64/thunderx.md
index 922df39..058713a 100644
--- a/gcc/config/aarch64/thunderx.md
+++ b/gcc/config/aarch64/thunderx.md
@@ -156,7 +156,7 @@
(define_insn_reservation "thunderx_fcmp" 3
(and (eq_attr "tune" "thunderx")
- (eq_attr "type" "fcmps,fcmpd"))
+ (eq_attr "type" "fcmps,fcmpd,fccmps,fccmpd"))
"thunderx_pipe1")
(define_insn_reservation "thunderx_fmul" 6
diff --git a/gcc/config/arm/cortex-a53.md b/gcc/config/arm/cortex-a53.md
index c1eeedb..fc60bc2 100644
--- a/gcc/config/arm/cortex-a53.md
+++ b/gcc/config/arm/cortex-a53.md
@@ -508,8 +508,8 @@
(define_insn_reservation "cortex_a53_fpalu" 5
(and (eq_attr "tune" "cortexa53")
(eq_attr "type" "ffariths, fadds, ffarithd, faddd, fmov,
- f_cvt, fcmps, fcmpd, fcsel, f_rints, f_rintd,
- f_minmaxs, f_minmaxd"))
+ f_cvt, fcmps, fcmpd, fccmps, fccmpd, fcsel,
+ f_rints, f_rintd, f_minmaxs, f_minmaxd"))
"cortex_a53_slot_any,cortex_a53_fp_alu")
(define_insn_reservation "cortex_a53_fconst" 3
diff --git a/gcc/config/arm/cortex-a57.md b/gcc/config/arm/cortex-a57.md
index 0d28951..f4c112c 100644
--- a/gcc/config/arm/cortex-a57.md
+++ b/gcc/config/arm/cortex-a57.md
@@ -716,7 +716,7 @@
(define_insn_reservation "cortex_a57_fp_cmp" 7
(and (eq_attr "tune" "cortexa57")
- (eq_attr "type" "fcmps,fcmpd"))
+ (eq_attr "type" "fcmps,fcmpd,fccmps,fccmpd"))
"ca57_cx2")
(define_insn_reservation "cortex_a57_fp_arith" 4
diff --git a/gcc/config/arm/exynos-m1.md b/gcc/config/arm/exynos-m1.md
index 0448073..973c8a9 100644
--- a/gcc/config/arm/exynos-m1.md
+++ b/gcc/config/arm/exynos-m1.md
@@ -823,6 +823,11 @@
(eq_attr "type" "fcmps, fcmpd"))
"em1_nmisc")
+(define_insn_reservation "exynos_m1_fp_ccmp" 7
+ (and (eq_attr "tune" "exynosm1")
+ (eq_attr "type" "fccmps, fccmpd"))
+ "em1_st, em1_nmisc")
+
(define_insn_reservation "exynos_m1_fp_sel" 4
(and (eq_attr "tune" "exynosm1")
(eq_attr "type" "fcsel"))
diff --git a/gcc/config/arm/types.md b/gcc/config/arm/types.md
index 321ff89..25f79b4 100644
--- a/gcc/config/arm/types.md
+++ b/gcc/config/arm/types.md
@@ -70,6 +70,7 @@
; f_rint[d,s] double/single floating point rount to integral.
; f_store[d,s] double/single store to memory. Used for VFP unit.
; fadd[d,s] double/single floating-point scalar addition.
+; fccmp[d,s] From ARMv8-A: floating-point conditional compare.
; fcmp[d,s] double/single floating-point compare.
; fconst[d,s] double/single load immediate.
; fcsel From ARMv8-A: Floating-point conditional select.
@@ -582,6 +583,8 @@
f_stores,\
faddd,\
fadds,\
+ fccmpd,\
+ fccmps,\
fcmpd,\
fcmps,\
fconstd,\
diff --git a/gcc/config/arm/xgene1.md b/gcc/config/arm/xgene1.md
index 8dfd8a1..b7aeac6 100644
--- a/gcc/config/arm/xgene1.md
+++ b/gcc/config/arm/xgene1.md
@@ -154,7 +154,7 @@
(define_insn_reservation "xgene1_fcmp" 10
(and (eq_attr "tune" "xgene1")
- (eq_attr "type" "fcmpd,fcmps"))
+ (eq_attr "type" "fcmpd,fcmps,fccmpd,fccmps"))
"xgene1_decode1op,xgene1_fsu+xgene1_fcmp*3")
(define_insn_reservation "xgene1_fcsel" 3
--
2.6.3